Skip to content

feat(flexlb): batch scheduling infrastructure, strategy refactor, and error model hardening#1138

Open
wzy-99 wants to merge 41 commits into
feat/dsv4_on_devfrom
feat/flexlb_base
Open

feat(flexlb): batch scheduling infrastructure, strategy refactor, and error model hardening#1138
wzy-99 wants to merge 41 commits into
feat/dsv4_on_devfrom
feat/flexlb_base

Conversation

@wzy-99

@wzy-99 wzy-99 commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR introduces FlexLB batch scheduling infrastructure with SLO-based admission control, strategy refactoring, and error model hardening.

Key Changes

Batch Scheduling Infrastructure

  • Batch scheduling with SLO-based admission control
  • FixedWindowBatcherAlgorithm with configurable batch window and SLO budgets
  • BatcherContext with TOCTOU-safe dropHead() and sortedItems() caching
  • FlexlbBatchScheduler with inflight eviction and cancel race protection

Strategy Refactor

  • Extract BatchItem as standalone class to decouple scheduler
  • Isolate WorkerEndpoint state (volatile WorkerStatus for cross-thread visibility)
  • WorkerEndpoint as sole Worker abstraction

Error Model Hardening (C++ engine)

  • Error reporting (MALLOC_FAILED) in GenerateStream lifecycle methods
  • Request timeout detection (checkTimeoutWithoutLock) in GenerateStream
  • ResponseBuffer queue capacity limit (kMaxQueueSize=1000) to prevent OOM
  • Error-bearing unit cleanup in FIFOScheduler waiting queue
  • Pickle backward compatibility for RuntimeConfig/PDSepConfig (size >= N)
  • Pass engine request_id through to API response aux_info

Review Fixes (Round 3)

  • GrpcWorkerStatusRunner: consecutive failure counter (max 3) before marking worker dead
  • worker_status.py: support str role values from JSON config path
  • ConfigService: try-catch for FLEXLB_CONFIG JSON parse fallback
  • FlexlbConfig: NumberFormatException handling
  • BatcherContext: TOCTOU fix with sortedItems caching
  • FixedWindowBatcherAlgorithm: force-dispatch to avoid busy-wait dead loop
  • WorkerEndpoint: volatile WorkerStatus

Test Coverage

  • Unit tests: PrefillEndpoint, DefaultBatchDispatcher, InflightEvictor, PrefillTimePredictor

Previous Reviews

  • Round 1 (16:48 6/23): P0/3 + P1/10 — addressed in 2640241
  • Round 2 (02:11 6/24): P1/10 — addressed
  • Round 3 (03:51 6/24): P0/2 + P1/15 — remaining issues fixed in this PR

@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/1 · P1/0 · P2/1 · P3/0

Blocking Issues

P0

  • GrpcWorkerStatusRunner 缺少 AtomicLong import,编译失败 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/GrpcWorkerStatusRunner.java:43
    • 建议:在 import 区域添加 import java.util.concurrent.atomic.AtomicLong;(与 java.util.Map 等 JDK import 放在一起)。

Non-blocking Suggestions

P2

  • getattr(RoleType, role) 大小写敏感且异常类型与契约不一致 @ rtp_llm/server/worker_status.py:81
    • 建议:改为 getattr(RoleType, role.upper(), None) 并对 None 结果抛出明确的 ValueError,或用 try/except AttributeError 包装并 raise ValueError(...) from e。

Checklist Violations (3 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue getattr(RoleType, role) 大小写敏感且异常类型与契约不一致
    worker_status.py 中 getattr(RoleType, role) 对无效字符串抛出 AttributeError 而非 ValueError,错误类型与 else 分支不一致。虽然 pydantic 会包装为 ValidationError,但错误消息不清晰。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → checklist-only
    连续失败计数器逻辑是关键行为变更(从单次失败判死改为 3 次连续失败),但 GrpcWorkerStatusCheckRunnerTest 仅覆盖成功路径,缺少失败容忍场景。Python 侧 validate_role str 分支在 schedule_meta_test.py 中已有覆盖。不升级为 issue,因改动较小且 GrpcWorkerStatusRunner 通常在集成层面验证。

Python Static-First Checklist

  • [P.A] 静态结构与类型纪律 — 禁止 getattr/setattr literal 访问 → issue getattr(RoleType, role) 大小写敏感且异常类型与契约不一致
    worker_status.py:81 使用 getattr(RoleType, role) 做枚举名称动态查找,属于 getattr 合理使用场景(动态变量 key),但对非法字符串抛出 AttributeError 而非 ValueError 的行为不理想。已在 issues 中报告。

Strengths

  • 连续失败计数器(consecutiveFailures + MAX_CONSECUTIVE_FAILURES=3)是合理的容错设计,避免单次 gRPC 超时或网络抖动误判 worker 死亡
  • include 路径从 model_rpc/ 修正为 cache/ 与 BUILD 依赖 recent_cache_key_window 同步修改,保持头文件位置与构建依赖一致
  • 成功响应后重置 consecutiveFailures.set(0) 的位置正确——放在 handleStatusResponse 开头,确保只有真正收到有效响应才重置

@wzy-99 wzy-99 force-pushed the feat/flexlb_base branch 2 times, most recently from 5dff119 to e5fa216 Compare June 24, 2026 11:21
@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/1 · P1/0 · P2/1 · P3/1

Blocking Issues

P0

  • consecutiveFailures 计数器因 Runner 实例每次新建而永远无法累积到阈值 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/GrpcWorkerStatusRunner.java:44
    • 建议:将 consecutiveFailures 计数器移到 WorkerStatus 对象上(生命周期跨同步周期),或在 EngineSyncRunner 中维护 Map<String, AtomicLong> 逐 worker 累积失败次数,构造 Runner 时传入。

Non-blocking Suggestions

P2

  • gRPC 异常路径重复上报两次错误指标 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/GrpcWorkerStatusRunner.java:91
    • 建议:在 launchGrpcStatusCheck 返回 null 后跳过 handleStatusResponse 中的 RESPONSE_NULL 上报,或在 handleStatusResponse 中区分「gRPC 异常导致的 null」和「正常返回但 body 为空」两种情况。

P3

  • Enum 查找建议使用 item 访问而非 getattr @ rtp_llm/server/worker_status.py:82
    • 建议:若 RoleType 是标准 Enum,改用 RoleType[role.upper()] 并将 except 改为 KeyError;若是 pybind11 枚举,添加注释说明使用 getattr 是有意为之。

Checklist Violations (7 fail / 69 total)

General Principles Checklist

  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue consecutiveFailures 计数器因 Runner 实例每次新建而永远无法累积到阈值
    GrpcWorkerStatusRunner.consecutiveFailures 是实例字段,但 EngineSyncRunner 每个同步周期新建 Runner 实例(line 139),导致计数器永远从 0 开始,永远达不到阈值 3。Worker liveness 检测失效——持续 gRPC 失败时 worker 永远不会被标记为 dead。
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue gRPC 异常路径重复上报两次错误指标
    gRPC 异常时 handleException 上报错误指标后返回 null,handleStatusResponse(null) 再次上报 RESPONSE_NULL,同一失败产生两条独立指标,干扰监控准确性。
  • [6.1] Architecture — 兼容性:公开 API/持久数据/配置/环境迁移安全 → checklist-only
    return_request_id 字段从 GenerateConfig/AuxInfo/GenerateOutputs 移除。该功能默认 false、commit 标注为 revert、未正式发布,pydantic BaseModel 默认忽略 extra fields,实际兼容性风险极低,不升级为 issue。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → checklist-only
    GrpcWorkerStatusCheckRunnerTest 覆盖了单次调用场景,但未测试连续失败累积。由于 Runner 是每次新建实例,现有测试框架无法覆盖跨周期累积——这是 P0 bug 的测试缺口,修复对象生命周期后需补充跨周期累积测试。
  • [6.1] Quality — Mega-PR 已拆分为独立变更 → checklist-only
    PR 混合了三个独立主题:return_request_id 移除、FlexLB 重构、C++ API 适配。各主题通过独立 commit 分隔,但理想情况下应拆为独立 PR。当前 commit 粒度清晰,不阻塞合入。
  • [6.1] Quality — PR description 说明动机与设计 → checklist-only
    从 commit 序列可推断 PR 动机,但 return_request_id 移除缺少显式迁移说明。因该功能默认 false 且未正式发布(commit 标注 revert),实际影响有限,不升级为 issue。

Python Static-First Checklist

  • [P.A] 静态结构与类型纪律 — 禁止 getattr/setattr literal 访问 → issue Enum 查找建议使用 item 访问而非 getattr
    worker_status.py:82 使用 getattr(RoleType, role.upper())。若 RoleType 是 pybind11 枚举则不支持 getitem,getattr 可接受;若是标准 Enum 应改用 RoleType[role.upper()]。当前因 except AttributeError 兜底不会出错,属 P3 风格改进。

Strengths

  • BatcherContext 的 dispatch/dropHead 从 queue.remove(item) 改为 remove(item),修复了 sortedCacheDirty 缓存失效标记未更新导致 sortedItems() 返回过期缓存的正确性缺陷
  • return_request_id 透传功能清理彻底:跨 9 个文件移除所有引用链,包括 GenerateConfig、ServerConfig、AuxInfo、GenerateOutputs、frontend_worker、openai_endpoint、custom_renderer、model_rpc_client、server_group_args
  • C++ cuda13 编译修复系统性解决 absl::string_view 隐式转换、include 路径(RecentCacheKeyWindow.h → cache/)和 BUILD 依赖问题
  • FlexLB 测试从旧 API (TaskStateEnum/waitingTaskList/localTaskMap) 完整迁移到新 API (TaskPhase/runningTaskList),新增空列表、null 列表、队列满等边界覆盖

@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/1 · P1/0 · P2/2 · P3/0

Blocking Issues

P0

  • GrpcWorkerStatusRunner 的 consecutiveFailures 计数器因实例每周期重建而永远无法累积 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/GrpcWorkerStatusRunner.java:44
    • 建议:将 consecutiveFailures 计数器移至 WorkerStatus 对象(已通过 getOrCreateWorkerStatus 跨周期复用),或将 GrpcWorkerStatusRunner 改为每个 worker 缓存复用同一实例。

Non-blocking Suggestions

P2

  • envValueIsFalse/envValueIsTrue 及相关环境变量检查函数在两个文件中重复定义 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:66
    • 建议:将 envValueIsTrue/envValueIsFalse 和 prefillTraceLogEnabled/prefillTheoryHitLogEnabled 提取到公共 utility header(如 rtp_llm/cpp/utils/EnvUtils.h),两处统一引用。
  • GrpcWorkerStatusRunner 连续失败阈值 MAX_CONSECUTIVE_FAILURES=3 硬编码且偏低 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/GrpcWorkerStatusRunner.java:45
    • 建议:通过 FlexlbConfig 使阈值可配置,默认值可适当提高以适应瞬时网络抖动;或至少在常量旁注释选择 3 的依据。

Checklist Violations (9 fail / 88 total)

General Principles Checklist

  • [6.1] Software Engineering — DRY:重复非平凡逻辑被抽取或显式复用 → issue envValueIsFalse/envValueIsTrue 及相关环境变量检查函数在两个文件中重复定义
    envValueIsFalse/envValueIsTrue 及 prefillTraceLogEnabled/prefillTheoryHitLogEnabled 在 PrefillRpcServer.cc:60-103 和 PrefillCacheHitMetricsReporter.cc:22-60 完全重复定义,共 4 个函数。
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue GrpcWorkerStatusRunner 的 consecutiveFailures 计数器因实例每周期重建而永远无法累积
    GrpcWorkerStatusRunner 的 consecutiveFailures 是实例字段,但 EngineSyncRunner 每周期创建新实例(:139-142),计数器无法累积,setAlive(false) 永远不执行。
  • [6.1] Architecture — 兼容性:公开 API/持久数据/配置/环境迁移安全 → checklist-only
    return_request_id/enable_return_request_id 字段直接删除无 deprecation warning。该特性默认 false 且为内部调试用,移除对大多数部署无影响,风险较低,不升级为 issue。
  • [6.1] Quality — 逻辑变更未混入无关格式化 → checklist-only
    PR 混合多个独立特性方向但各方向内部逻辑自洽,不是格式化噪声混入。在 feature branch 上下文中可接受,不升级为 issue。
  • [6.1] Quality — Mega-PR 已拆分为独立变更 → checklist-only
    28 个文件跨 Python/C++/Java 三语言含 5+ 个独立方向。建议拆分,但在 feature branch 合并场景下各方向存在关联,可接受,不升级为 issue。

RTP-LLM Checklist

  • [I] 代码质量 — 同一功能用统一工具函数 → issue envValueIsFalse/envValueIsTrue 及相关环境变量检查函数在两个文件中重复定义
    envValueIsFalse/envValueIsTrue 及 prefillTraceLogEnabled/prefillTheoryHitLogEnabled 在 PrefillRpcServer.cc 和 PrefillCacheHitMetricsReporter.cc 完全重复定义(共 4 个函数)。
  • [A] 兼容性与配置 — 环境变量/命令行参数/proto 重命名有向后兼容 fallback → checklist-only
    --enable_return_request_id CLI 参数及对应 ServerConfig 字段直接删除。该特性默认 false 且为内部调试用,移除对大多数部署无影响,与兼容性 checklist 同因,不升级为 issue。
  • [B] 正确性与逻辑 — 逻辑错误、off-by-one、null/zero 检查 → issue GrpcWorkerStatusRunner 的 consecutiveFailures 计数器因实例每周期重建而永远无法累积
    GrpcWorkerStatusRunner.consecutiveFailures 是实例字段,EngineSyncRunner 每周期 new 实例(:139-142),计数器永远从 0 开始,setAlive(false) 永远不执行。
  • [B] 正确性与逻辑 — 状态标志有完整 set/reset 生命周期 → issue GrpcWorkerStatusRunner 的 consecutiveFailures 计数器因实例每周期重建而永远无法累积
    consecutiveFailures 的 set(0) 和 incrementAndGet 无法跨周期配合,因 EngineSyncRunner 每周期重建 GrpcWorkerStatusRunner 实例。

Strengths

  • return_request_id 全链路清理彻底:GenerateConfig、ServerConfig、AuxInfo、GenerateOutputs、前端、OpenAI endpoint、custom_renderer、model_rpc_client 所有生产者和消费者均同步移除,无遗留引用
  • BatcherContext.dispatch/dropHead 从 queue.remove() 改为 this.remove() 正确修复了 sortedCacheDirty 标志未更新的缓存失效 bug
  • FIFOScheduler group isolation 测试覆盖全面,含 size2/3/4、group+single 混合、多 group 隔离、交错入队等边界场景

@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: LGTM

Summary: P0/0 · P1/0 · P2/1 · P3/1

lgtm ready to ci

Non-blocking Suggestions

P2

  • FlexLB 默认 batch 算法从 slo_budget 改为 fixed_window,影响现有部署 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/FlexlbConfig.java:364
    • 建议:在 PR description 或 release notes 中明确标注此默认值变更及其影响,确认所有现有部署已评估或显式设置了 flexlbBatchAlgorithm。如果 fixed_window 已经过充分验证且确实更适合默认场景,可保留此变更。

P3

  • envValueIsFalse 在 PrefillRpcServer.cc 和 PrefillCacheHitMetricsReporter.cc 中重复定义 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:63
    • 建议:将 envValueIsTrue/envValueIsFalse 提取到共享 utility header(如 rtp_llm/cpp/utils/EnvUtils.h),两个源文件统一引用。

Checklist Violations (7 fail / 103 total)

General Principles Checklist

  • [6.1] Software Engineering — DRY:重复非平凡逻辑被抽取或显式复用 → issue envValueIsFalse 在 PrefillRpcServer.cc 和 PrefillCacheHitMetricsReporter.cc 中重复定义
    envValueIsFalse/envValueIsTrue 在 PrefillRpcServer.cc(文件作用域)和 PrefillCacheHitMetricsReporter.cc(匿名命名空间)中各有完整独立实现,逻辑完全相同,应提取到共享工具头文件。
  • [6.1] Architecture — 兼容性:公开 API/持久数据/配置/环境迁移安全 → issue FlexLB 默认 batch 算法从 slo_budget 改为 fixed_window,影响现有部署
    FlexlbConfig.flexlbBatchAlgorithm 默认值从 slo_budget 变更为 fixed_window,未显式配置的现有部署将静默切换批处理行为,丧失 SLO deadline tracking 和 EMA 估计。
  • [6.1] Tests — 被删除测试有等价替代覆盖 → checklist-only
    FlexlbBatchSchedulerTest 删除了 processQueue_dispatches_when_batch_size_reached 测试,slo_budget 仍作为 WorkerBatcher 的 fallback 算法存在(第 49 行)。WorkerStatusTest 和 ConfigServiceTest 删除的测试已由 PrefillResourceMeasureTest 等替代,但 slo_budget batch size 触发路径覆盖有所降低。影响有限因 slo_budget 已非默认算法。

RTP-LLM Checklist

  • [A] 兼容性与配置 — 环境变量/命令行参数/proto 重命名有向后兼容 fallback → checklist-only
    --enable_return_request_id CLI 参数和 ENABLE_RETURN_REQUEST_ID 环境变量被完全移除,无 deprecation warning 或 fallback。使用此参数的部署脚本将因 argparse unrecognized argument 启动失败。影响有限(默认 False 且标注为 internal debugging only),但严格来说缺少向后兼容 fallback。
  • [A] 兼容性与配置 — 默认值变更已评估对现有用户影响 → issue FlexLB 默认 batch 算法从 slo_budget 改为 fixed_window,影响现有部署
    FlexlbConfig.flexlbBatchAlgorithm 默认从 slo_budget → fixed_window。环境变量可覆盖,但未在 PR 中标注影响评估。
  • [H] 测试与 CI — 测试覆盖充分:大重构等价覆盖,新功能端到端测试 → checklist-only
    FIFOScheduler group 隔离新增多个测试覆盖全面,Java 测试重写适配新 API。但 slo_budget batcher 的 batch size 触发测试(processQueue_dispatches_when_batch_size_reached)被删除,slo_budget 仍作为 WorkerBatcher 的 fallback 算法存在,该路径覆盖有所降低。影响有限因 slo_budget 已非默认算法。
  • [I] 代码质量 — 同一功能用统一工具函数 → issue envValueIsFalse 在 PrefillRpcServer.cc 和 PrefillCacheHitMetricsReporter.cc 中重复定义
    envValueIsFalse/envValueIsTrue 在 PrefillRpcServer.cc(文件作用域)和 PrefillCacheHitMetricsReporter.cc(匿名命名空间)中各有完整独立实现,逻辑完全相同。应提取到共享工具头文件。

Strengths

  • PrefillRpcServer EnqueueGroup 中 slots_ptr lambda 捕获修复:正确捕获 shared_ptr<vector>,防止 outer lambda 销毁后 inner lambda 中的裸指针悬空(use-after-free 修复)
  • BatcherContext.remove() 修复:将 queue.remove(item) 改为 remove(item),确保移除元素时正确设置 sortedCacheDirty = true,避免排序缓存返回已删除的过时数据
  • GrpcWorkerStatusRunner 引入连续失败计数器(AtomicLong),要求连续 3 次 gRPC 失败才将 worker 标记为 dead,避免单次网络抖动导致节点误下线
  • FIFOScheduler group isolation 新增全面测试用例,覆盖 group size 2/3/4、group 与 single 不混排、多 group 隔离、interleaved 等边界场景
  • GenerateStream.cc 将错误的 result.status().message() 改为正确的 result.message(),并显式构造 std::string 避免 string_view 拼接歧义
  • return_request_id 特性清理彻底:config、server args、frontend、openai endpoint、renderer、datatypes 全链路联动删除,无悬挂引用

@github-actions

Copy link
Copy Markdown

CI dispatcher could not find a native build run for HEAD SHA c8af443e.

This can happen if the PR was opened before the CI architecture change, or if the original run was deleted.

To fix: push any commit (even empty: git commit --allow-empty -m "trigger CI" && git push) to create a native build run, then re-approve or post lgtm ready to ci.

@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/0 · P1/3 · P2/6 · P3/2

Blocking Issues

P1

  • isNonBatchPath 逻辑遗漏 AUTO 模式下不符合 batch 条件的请求 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedPrefillStrategy.java:274
    • 建议:CostBasedPrefillStrategy 只能被非 batch 路径到达(batch 路径走 FlexlbBatchScheduler),因此 isNonBatchPath 应改为 return ctx.getScheduleMode() != ScheduleModeEnum.BATCH; 或直接去掉条件始终 commit。当前逻辑在默认配置下(batchEnabled=true, AUTO)会导致 maxNewTokens<=1 等不合格 batch 请求无 inflight 追踪,cost-based scoring 失准引起热点。
  • GrpcWorkerStatusRunner.getRole().name() 可能 NPE @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/GrpcWorkerStatusRunner.java:111
    • 建议:改为防空: String roleName = newWorkerStatus.getRole() != null ? newWorkerStatus.getRole().name() : "UNKNOWN"; 或在 RoleTypeProtoConverter 中为 default case 返回一个 fallback 枚举值而非 null。NPE 会导致该 worker 的状态同步线程中断,后续所有健康检查失效。
  • RoleAddr.validate_role 不再接受 str,但 serialize_role 仍输出 str,JSON 往返反序列化会失败 @ rtp_llm/config/generate_config.py:53
    • 建议:validate_role 应同时接受 str,与 ServerStatus.validate_role (worker_status.py:74-86) 保持一致: 增加 elif isinstance(v, str): return getattr(RoleType, v.upper())。或将 serialize_role 改为输出 int (role.value),使序列化格式与验证器匹配。

Non-blocking Suggestions

P2

  • 非批次路径 inflight 无上限,可能在高 QPS 下放大 realWaitTimeMs() 迭代开销 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedPrefillStrategy.java:253
    • 建议:考虑为非批次路径添加类似 maxInflightBatchesPerWorker 的准入控制,或在 doSelect 内缓存 realWaitTimeMs() 结果避免同一轮决策中对同一 endpoint 重复迭代(当前单轮最多调用 5 次)。短期内 TTL=5min 兜底可防止无限增长,但在 engine crash + 高 QPS 场景下 5 分钟内可能积累大量 entry。
  • realWaitTimeMs() 在单次 doSelect 中对同一 endpoint 重复调用多达 5 次 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedPrefillStrategy.java:142
    • 建议:在 doSelect 开头对每个 endpoint 计算一次 waitMs 并缓存到局部 Map<PrefillEndpoint, Long>,后续引用缓存值。这样每个 endpoint 只遍历一次 inflightBatches,减少 N5 → N1 次 map 迭代。注意:此为既有模式,本 PR 通过增加非批次 inflight 条目使其影响略有放大。
  • CostBasedPrefillStrategy 测试未覆盖非 batch 路径 inflight 追踪 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedPrefillStrategy.java:253
    • 建议:添加测试用例:设置 scheduleMode=DIRECT 或 flexlbBatchEnabled=false,select 后验证 ep.realWaitTimeMs() > 0 / inflightBatches 包含 requestId 条目;并验证 rollBack 后条目被清除。
  • calibrate 中非 batch 请求 warn 日志在 isNonBatchPath 缺陷下会频繁误报 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/endpoint/PrefillEndpoint.java:147
    • 建议:修复 isNonBatchPath 后此问题自然消失。如果保持现状,应将 warn 降为 debug 以避免日志刷屏(因为 non-batch 请求未被 commit 到 inflight map 是一个已知的'设计'行为)。
  • is_backend_service_ready 硬编码返回 True,前端健康检查失效 @ rtp_llm/server/backend_rpc_server_visitor.py:549
    • 建议:即便 FlexLB 接管路由,健康检查仍应验证 FlexLB master 可达性,否则所有请求在运行时才发现路由失败。建议至少保留一个轻量级的 FlexLB 连通性检查,或在注释中说明上游 K8s probe 如何覆盖此场景。同时移除注释掉的旧代码,避免 dead code。
  • ConfigService 解析 FLEXLB_CONFIG 失败时静默回退到默认配置 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/ConfigService.java:37
    • 建议:配置解析失败回退到默认值可能导致生产环境用错误配置静默运行(如 TTL、batch size 等全用默认值)。建议改为 fail-fast 抛出异常,或至少上报一个告警 metric。

P3

  • psutil 从延迟导入改为模块顶层导入,影响 45 个导入者的启动速度 @ rtp_llm/utils/util.py:12
    • 建议:保持 psutil 的延迟导入(在 stop_server 内部 import),或移至仅 stop_server 使用的独立模块,避免所有导入 util.py 的模块无条件加载这个 C 扩展。
  • psutil 顶层 import 无 ImportError 保护 @ rtp_llm/utils/util.py:13
    • 建议:psutil 是必需依赖且已列入 requirements,顶层 import 是合理的模式改进。无需改动,仅注意确保 psutil 在所有测试/生产环境的 requirements 中。

Checklist Violations (1 fail / 56 total)

General Principles Checklist

  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue CostBasedPrefillStrategy 测试未覆盖非 batch 路径 inflight 追踪
    isNonBatchPath 在默认配置下返回 false,导致 CostBasedPrefillStrategyTest 从未触发 commitBatch 路径。非 batch inflight 追踪的核心新逻辑缺乏有效测试覆盖。

Strengths

  • 将 inflight TTL 从 1 小时降至 5 分钟,显著减少 engine crash 后 stale entry 对 realWaitTimeMs 估算的扭曲时间窗口
  • endpointRegistry.evictExpiredAll() 独立调度,解耦了 BatchScheduler 和 EndpointRegistry 的生命周期职责
  • 非批次路径正确实现了 inflight 预留(commitBatch)+ 回滚(releaseBatch),使 load-aware scoring 对所有路由路径生效
  • calibrate() 中对 batchId<0 的非批次请求立即清理 inflight,避免等到 TTL 才释放
  • TTL eviction 从 FlexlbBatchScheduler 解耦到 EndpointRegistry 自身的 @scheduled 方法,职责更清晰
  • 非 batch 请求的 inflight 清理逻辑(calibrate Phase 1)设计思路正确:按 requestId 作为 key 直接移除
  • rollBack 方法从空实现改为实际释放 inflight 预留,修复了路由失败时的 inflight 泄漏
  • TTL 从 1h 降低到 5min 更合理地平衡了安全网延迟与 realWaitTimeMs 准确度

…ng, and PrefillRpcServer metrics

Combined from:
- cc55836 feat: sync flexlb batch async fetch
- b422c6c feat(flexlb): batch scheduling, strategy refactor, error model hardening, and test coverage
- 89adee8 feat(PrefillRpcServer): enhance thread pool management with worker lambda pool and metrics
- 909cfe5 feat(util): add requests library import for HTTP functionality
@wzy-99 wzy-99 force-pushed the feat/flexlb_base branch from 5c9f37a to 63e65ed Compare June 26, 2026 03:32
@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/0 · P1/11 · P2/16 · P3/2

Blocking Issues

P1

  • setPrefillCoefficients 解析中途失败导致系数半更新 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/FlexlbConfig.java:314
    • 建议:先解析到临时变量,全部成功后再赋值。parts.length<3 时 log.warn。
  • enqueue() 硬编码 group_id=-1,丢弃 stream 的 group 语义 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:116
    • 建议:enqueue() 应读取 stream->groupId(),或对 isGroup() stream 拒绝入队并要求使用 enqueueGroup()。
  • admitWaitingUnits 清除 errored unit 时未 finish 组内无错 stream,导致永久挂起 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:217
    • 建议:erase unit 前对所有 stream 调用 reportError + finish()。
  • master_client_test.py 使用旧 HTTP 签名,与新 gRPC API 不匹配 @ rtp_llm/server/test/master_client_test.py:46
    • 建议:重写 _CaptureMasterClient 匹配新 gRPC 签名,修复所有断言。
  • RpcCpuTpBroadcaster 使用 DP 过滤的地址列表,dp_size>1 时 CHECK 必崩 @ rtp_llm/cpp/normal_engine/NormalExecutor.cc:115
    • 建议:传入 all_worker_grpc_addrs(本 PR 新增字段,world_size 大小)而非 worker_grpc_addrs。
  • getEngineScheduleInfo 持 shared_lock 却写入 entry.task_info.phase,数据竞争 @ rtp_llm/cpp/model_rpc/RpcServerRuntimeMeta.h:33
    • 建议:拷贝到局部变量再修改:auto copy = entry.task_info; copy.phase = ...; push_back(copy);
  • pushTask 后 use-after-move,fallback 线程拿到空 lambda @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:1853
    • 建议:先传拷贝给 pushTask:auto err = pool->pushTask(finish_lambda); if (err) std::thread(std::move(finish_lambda)).detach();
  • activelyNotifyParticipants 通知条件反转,远程节点永远收不到 master 切换通知 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/consistency/ZookeeperMasterElectService.java:371
    • 建议:改为 !localIp.equals(participant.getId()),与注释 'Only notify non-master participants' 一致。
  • waitForLeadershipTransfer while(true) 无退出上限,ZK 分区时 shutdown 永久阻塞 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/consistency/ZookeeperMasterElectService.java:216
    • 建议:增加最大等待次数(如 30 次 = 30 秒),超过后 warn + 强制退出。
  • ResourceMeasure 接口重构导致 RandomStrategy 资源检查失效 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/resource/ResourceMeasure.java:22
    • 建议:DecodeResourceMeasure/PrefillResourceMeasure 需 @OverRide isResourceAvailable(WorkerEndpoint),内部 instanceof 委托到类型化方法。
  • RoleAddr validator 删除 string 支持但 serializer 仍输出 string @ rtp_llm/config/generate_config.py:54
    • 建议:validator 应同时支持 str:if isinstance(v, str): return RoleType[v]。或 serializer 改为输出 int。

Non-blocking Suggestions

P2

  • getParsedSloBuckets 无同步保护、返回可变 List、静默吞异常 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/FlexlbConfig.java:467
    • 建议:setCostSloBuckets 中 eager init;返回 unmodifiableList;catch 中 log.warn。
  • WorkerStatus.updateFromResponse 混用 AtomicLong 和非 volatile 字段无同步 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/dao/master/WorkerStatus.java:53
    • 建议:updateFromResponse 内加 lock,或将高频读取字段改为 volatile。
  • applyTrafficPolicyOverride 缺 try-catch,异常配置导致启动崩溃 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/ConfigService.java:123
    • 建议:仿 FLEXLB_CONFIG 路径加 try-catch,解析失败时 log.error 并跳过覆盖。
  • canAdmitUnit 对 group unit 完全跳过 token 预算检查 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:200
    • 建议:对 group unit 也应检查 token 预算,或添加注释说明此为有意行为并记录 WARNING。
  • RuntimeConfig/PDSepConfig pickle 不向后兼容 @ rtp_llm/cpp/pybind/ConfigInit.cc:1305
    • 建议:接受旧长度(13/20),新字段可选读取并使用默认值。参考 ProfilingDebugLoggingConfig 的做法。
  • ResponseBufferWriter::Write 队列满时静默丢弃最旧消息 @ rtp_llm/cpp/model_rpc/ResponseBuffer.cc:122
    • 建议:首次丢弃时 log WARNING(限频),或设置 error_status 通知消费端。
  • countStreams() 每次遍历所有 unit,schedule() 热路径调用 8 次 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:169
    • 建议:维护增量计数器(enqueue 时 +=, erase 时 -=),或在 schedule() 开头缓存一次。
  • RpcCpuTpBroadcaster inbox 无大小上限 @ rtp_llm/cpp/distribute/RpcCpuTpBroadcaster.cc:281
    • 建议:添加 inbox_ 大小上限(如 tp_size * 128),超限时 reject 并 LOG_WARNING。
  • stopAsyncResponseWorkers 超时后 force reset count 可能下溢 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:783
    • 建议:用 atomic + stop flag,finishAsyncResponseWorker 中检查 stop flag。
  • ExpirationCleaner 移除 worker 后未清理 EndpointRegistry @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/schedule/ExpirationCleaner.java:49
    • 建议:移除 worker 后通知 EndpointRegistry 清理对应 endpoint(close batcher + 移除 map entry)。
  • ensureEndpoint 异常中断整个 sync 轮次 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/EngineSyncRunner.java:233
    • 建议:改为 Logger.warn + continue,只跳过该 worker 不中断循环。
  • LBStatusConsistencyService 静态线程池未关闭 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/consistency/LBStatusConsistencyService.java:27
    • 建议:在 destroy() 中增加 SCHEDULED_EXECUTOR_SERVICE.shutdownNow()。
  • GenerateConfig 删除 force_batch 字段可能破坏外部 JSON 调用方 @ rtp_llm/config/generate_config.py:175
    • 建议:确认无外部消费者,或添加向后兼容别名 / model_config extra='ignore'。
  • applyEnvironmentOverrides 直接 field.set() 绕过自定义 setter @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/ConfigService.java:89
    • 建议:对存在自定义 setter 的字段改用 setter 调用。
  • EnqueueBatch 热路径同步执行 response_registry_.gc() @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:1597
    • 建议:删除此行,后台 GC 线程已覆盖。
  • appendPrefillTheoryHitLogLine 无轮转机制,日志文件无限增长 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:455
    • 建议:添加文件大小检查和轮转,或通过现有日志框架输出。

P3

  • NormalExecutor 和 MtpExecutor 中 readEnvFlagOnce 重复定义 @ rtp_llm/cpp/normal_engine/NormalExecutor.cc:52
    • 建议:提取到共用 utils 头文件。
  • GatherBatchScheduler 重复定义父类已有的方法 @ rtp_llm/cpp/engine_base/schedulers/GatherBatchScheduler.h:107
    • 建议:将父类方法改为 protected,子类直接复用。

Checklist Violations (7 fail / 56 total)

General Principles Checklist

  • [6.1] Software Engineering — LSP:子类/重写保持基类契约 → issue ResourceMeasure 接口重构导致 RandomStrategy 资源检查失效
    ResourceMeasure 默认方法与子类 overload 形成隐式契约断裂,RandomStrategy 走默认实现跳过资源检查
  • [6.1] Software Engineering — DRY:重复非平凡逻辑被抽取或显式复用 → issue NormalExecutor 和 MtpExecutor 中 readEnvFlagOnce 重复定义
    readEnvFlagOnce/readEnvIntOnce 在 NormalExecutor.cc 和 MtpExecutor.cc 中重复定义;GatherBatchScheduler 重复父类方法
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue admitWaitingUnits 清除 errored unit 时未 finish 组内无错 stream,导致永久挂起
    admitWaitingUnits 清除 errored unit 时不 finish 组内无错 stream;stopAsyncResponseWorkers 超时后 count 下溢
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue ensureEndpoint 异常中断整个 sync 轮次
    ensureEndpoint 异常中断整个 sync 轮次而非 skip + continue
  • [6.1] Architecture — 兼容性:公开 API/持久数据/配置/环境迁移安全 → issue RuntimeConfig/PDSepConfig pickle 不向后兼容
    RuntimeConfig/PDSepConfig pickle 不兼容旧版本;GenerateConfig 删除 force_batch 字段;RoleAddr serializer/validator 不一致
  • [6.1] Tests — 被删除测试有等价替代覆盖 → issue master_client_test.py 使用旧 HTTP 签名,与新 gRPC API 不匹配
    master_client_test.py 使用旧 HTTP API 签名,与新 gRPC 接口不匹配,测试必定失败

RTP-LLM Checklist

  • [I] 代码质量 — 同一功能用统一工具函数 → issue NormalExecutor 和 MtpExecutor 中 readEnvFlagOnce 重复定义
    readEnvFlagOnce/readEnvIntOnce 在 NormalExecutor.cc 和 MtpExecutor.cc 重复定义

Strengths

  • GenerateStateMachine 重构为 GenerateStream 上清晰的生命周期方法 (prepare/isReady/activate/advance/finish),大幅提升可维护性
  • 新增 ScheduleUnit 抽象统一管理 group 和 single stream 调度,消除旧 evaluateWaitingStreams 的复杂 group-gathering 逻辑
  • WorkerStatus 移除 localTaskMap ConcurrentHashMap 及其状态机迭代,简化为直接吸收 gRPC 响应
  • Logger 从自定义 log level + lambda 分配改为直接委托 SLF4J,消除每次调用的不必要对象分配
  • ConfigService 新增 FLEXLB_CONFIG 解析 try-catch 兜底,异常 JSON 不再导致启动失败

wzy-99 added 6 commits June 26, 2026 14:43
…e + 3 P1 fixes

- P0: move consecutiveFailures from GrpcWorkerStatusRunner instance field to
  WorkerStatus object, so the counter persists across sync cycles instead of
  being reset every time a new Runner is created
- P1: fix isNonBatchPath to use != BATCH instead of == DIRECT, so AUTO-mode
  requests that fail batch qualification also get inflight tracking
- P1: add null guard for newWorkerStatus.getRole().name() to prevent NPE
  crashing the sync thread
- P1: add str branch in generate_config.py validate_role for JSON round-trip
Add per-request INFO-level event logs across engine/scheduler/executor/
dispatcher layers for complete request traceability (prefill → decode →
finish). All events carry trace_id via streamLogTag() formatted as
'trace_id=XXX req_id=YYY'.

Changes:
- GenerateStream.h: implement streamLogTag() inline with trace_id+req_id
- FIFOScheduler.cc: add request_activated (waiting→running + loading→
  running) and request_finished (cleanup) logs
- NormalExecutor.cc: add trace_id to prefill_batch_begin; add new
  decode_step_begin event for decode batch visibility
- NormalOutputDispatcher.cc: add first_token + decode_finished per-request
  events (compatible with dispatchOutputAsync path)
- frontend_server.py: add request_arrival + request_completion logs

Design: all events use RTP_LLM_ACCESS_LOG_INFO (no [RANK][file:line]
prefix); INFO only at batch boundaries/state transitions; trace_id
coverage on all per-request events.
P1 #10: ResourceMeasure 重构导致 RandomStrategy 资源检查失效
  - DecodeResourceMeasure/PrefillResourceMeasure 添加 @OverRide isResourceAvailable(WorkerEndpoint) 桥接方法
  - 通过 instanceof 委托到类型化方法,修复之前仅走接口 default 方法(只查 isAlive())的 bug

P1 #8: activelyNotifyParticipants 通知条件反转
  - 修正 localIp.equals 为 !localIp.equals,确保远程节点能收到 master 切换通知

P1 #9: waitForLeadershipTransfer 无退出上限
  - 新增 MAX_WAIT_COUNT=30 最大等待保护,超时后 warn + 强制退出

P2 #3: applyTrafficPolicyOverride 缺 try-catch
  - JsonUtils.toObject 包裹 try-catch,防止异常 JSON 导致启动崩溃

P2 #12: LBStatusConsistencyService 静态线程池未关闭
  - 添加 @PreDestroy shutdown() 关闭 SCHEDULED_EXECUTOR_SERVICE
…ia PDSepConfig

- FlexlbConfig: flexlbBatchQueueMaxSize 64 → 1024
- PDSepConfig: add prefill_enqueue/worker_lambda/slot_pool_size fields (default 0 = formula)
- PrefillRpcServer: initThreadPools() reads from pd_sep_config instead of hardcoded/env var
- ConfigInit.cc: pybind registration + pickle for 3 new PDSepConfig fields
- pd_separation_group_args.py: CLI args for 3 thread pool sizes
…n in .h)

Commit f1df601 added an inline definition in GenerateStream.h but forgot
to remove the existing out-of-line implementation in GenerateStream.cc,
causing a redefinition error on cuda13_x86 builds.
- Add flexlbBatchFixedMaxInflightBatches config (default 0, disabled)
  to limit in-flight batches per prefill worker in fixed_window mode
- Rename SLO batcher config flexlbBatchMaxInflightBatchesPerWorker
  -> flexlbBatchSloMaxInflightBatches for naming consistency
- FixedWindowBatcherAlgorithm parks instead of dispatching when
  engine inflight batch count >= limit, preventing engine overload
@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/0 · P1/11 · P2/44 · P3/0

Blocking Issues

P1

  • streamLogTag 格式变更导致 Python/C++ 日志关联断裂 @ rtp_llm/cpp/engine_base/stream/GenerateStream.h:465
    • 建议:统一 C++/Python 的字段名和顺序。建议 C++ 保持 'request_id=' 而非缩写 'req_id=',或同步更新 rtp_llm/dash_sc/inference/servicer.py:108 的 stream_log_tag() 格式。同时保持字段顺序一致(两边都用 trace_id 在前或 request_id 在前)。
  • InflightEvictor 回调异常会中断后续所有 entry 的清理 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/InflightEvictor.java:52
    • 建议:在 evictExpired 循环内用 try-catch 包裹 onEvict.accept() 调用,确保单个 entry 的回调异常不影响其余 entry 的清理。例如:try { onEvict.accept(entry.getValue()); } catch (Exception e) { Logger.error(...); }
  • WorkerBatcher.shutdown() 与 waitForNonEmpty() 存在竞态导致请求丢失 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/WorkerBatcher.java:130
    • 建议:将 waitForNonEmpty() 改为 peek() + 短暂 sleep 或 LockSupport.parkNanos 的模式,避免从队列中临时移除 item。或者在 shutdown 中等待 workerThread 终止后再 drainTo,确保 take-put 原子窗口已关闭。
  • BatchInflight.lastSeenMs volatile 读-改-写存在竞态条件 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/endpoint/BatchInflight.java:76
    • 建议:将 lastSeenMs 改为 AtomicLong 并使用 updateAndGet(base -> Math.max(base, statusMs)),与 progressBaseMs 保持一致的模式。
  • BatcherContext.sortedItems() 缓存在 offer() 添加新项后不会失效 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/WorkerBatcher.java:74
    • 建议:在 WorkerBatcher.offer() 的 queue.add(item) 之后,通过 BatcherContext 暴露的方法将 sortedCacheDirty 设为 true;或者在 BatcherContext 中提供一个 offer() 包装方法,确保添加元素时缓存失效。
  • DefaultRouterTest 向全局静态 LoadBalanceStrategyFactory 注册 mock 但从不清理 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/DefaultRouterTest.java:93
    • 建议:在 tearDown() 中恢复 LoadBalanceStrategyFactory 的原始注册(或把注册行删掉,因为 mockStaticLoadBalanceStrategyFactory() 已通过反射覆写 DefaultRouter 内部 map,不需要注册到全局工厂)。另外 line 95 用 vitLoadBalancer 覆盖了 COST_BASED_PREFILL,应改为正确的 strategy enum 或直接移除这些 register 调用。
  • DefaultBatchDispatcherTest 缺少 @AfterEach 关闭 dispatcher,导致线程泄漏 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/DefaultBatchDispatcherTest.java:55
    • 建议:添加 @AfterEach void tearDown() { dispatcher.shutdown(); } 确保每个测试后线程池被关闭。
  • EngineWorkerStatusTest 使用内联清理而非 @AfterEach,断言失败时全局状态泄漏 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/sync/status/EngineWorkerStatusTest.java:81
    • 建议:添加 @AfterEach 方法统一清理所有 status map(decode/prefill/pdfusion/vit),移除每个测试末尾的内联 clear()。参考 DefaultRouterTest 的 tearDown() 写法。
  • CostBasedPrefillStrategyTest 缺少 @AfterEach 清理全局 prefill status map @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/strategy/CostBasedPrefillStrategyTest.java:46
    • 建议:添加 @AfterEach void tearDown() { EngineWorkerStatus.MODEL_ROLE_WORKER_STATUS.getPrefillStatusMap().clear(); } 确保测试隔离。
  • PDSepConfig unpickle 向后不兼容:旧版序列化数据无法反序列化 @ rtp_llm/cpp/pybind/ConfigInit.cc:1756
    • 建议:将最小 size 检查保留为 23(旧版最小值),对 index 23-25 使用条件赋值:if (t.size() > 23) c.prefill_enqueue_pool_size = t[23].cast<int64_t>(); 等。这样旧版数据可以用默认值 0 正常反序列化,保持滚动升级兼容。
  • setPrefillCoefficients 解析失败时会产生部分更新,导致系数不一致 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/FlexlbConfig.java:325
    • 建议:先解析到临时变量中,全部成功后再一次性赋值。当前实现中如 parts[1] 解析失败,costAlpha0 已被修改但 costAlpha1-5 保持旧值,违反了注释中 'Keep existing default values' 的约定,产生不一致的系数组合,影响 CostBasedPrefill 负载均衡决策。

Non-blocking Suggestions

P2

  • snprintf 固定 256 字节缓冲区可能截断长 trace_id @ rtp_llm/cpp/engine_base/stream/GenerateStream.h:466
    • 建议:使用 std::string 拼接或 fmt::format 代替固定缓冲区 snprintf,避免截断风险。如果坚持用固定缓冲区(为性能),至少对 trace_id 做长度截断并在截断时追加省略标记如 '...'。
  • request_finished 日志使用 %ld 打印 size_t 类型 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:289
    • 建议:将 %ld 改为 %zu(size_t 的正确格式符),或 static_cast 显式转换。虽然在 LP64 平台上正值不会出错,但不匹配的格式符是 UB。
  • FIFOScheduler 中 request_activated 日志代码块完全重复 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:258
    • 建议:提取为私有辅助函数,如 logStreamActivated(const GenerateStreamPtr& s),减少代码重复和 I-cache 占用。
  • backpressure park 时 predictor-based early dispatch 路径的候选列表分配被跳过,行为一致但注释过时 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/FixedWindowBatcherAlgorithm.java:29
    • 建议:删除第 29 行 '
    • No inflight-batch backpressure check.
    • ',因为该 PR 已经为 fixed_window 添加了 backpressure 检查。
  • [未确认-降级] WorkerBatcher 线程死亡后队列请求永久滞留 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/WorkerBatcher.java:42
    • 建议:方案一:在 PrefillEndpoint 或 EndpointRegistry 中加 watchdog 定期检查 workerThread.isAlive(),死亡后重建 batcher。方案二:将 waitForNonEmpty 改为 peek+sleep 模式避免 take-put 窗口。至少应在 UncaughtExceptionHandler 中将 stopped 置 true 并清空队列(调用 handler.onOfferFailure 给每个请求回错误)。
  • WorkerBatcher.offer() 队列大小检查存在 TOCTOU 竞态 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/WorkerBatcher.java:66
    • 建议:使用 AtomicInteger 计数器(offer 前 incrementAndGet,失败时 decrementAndGet),或使用信号量进行精确容量控制。对于调度场景,当前的近似限制可能可以接受——如果是,请在注释中说明是 best-effort。
  • EndpointRegistry 无删除/GC 机制,worker 轮换时内存无界增长 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/endpoint/EndpointRegistry.java:15
    • 建议:在 ExpirationCleaner 或 EndpointRegistry.scheduledEviction() 中联动检查:当 worker 从 workerStatusMap 移除后,也从 registry 中移除对应 endpoint(先 close 再 remove)。或在 EndpointRegistry 增加 removeStale() 方法定期调用。
  • DecodeEndpoint 过期 eviction 回调为空操作,无日志无指标 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/endpoint/DecodeEndpoint.java:26
    • 建议:在回调中添加 warn 级别日志(记录 requestId 和 kvTokens),并考虑增加 eviction counter 指标,便于生产环境发现 decode 确认丢失问题。
  • BatcherContext.sortedItems() 缓存在多线程下不安全 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/BatcherContext.java:97
    • 建议:将 sortedCache 也标记为 volatile,或将 sortedItems() 改为 synchronized。当前 headWaitMs/queueWaitMs 不调用 sortedItems(),所以实际风险受限——但如果未来算法变化引用了它就会触发。
  • LBStatusConsistencyService.SCHEDULED_EXECUTOR_SERVICE 是 static final 但 shutdown 是实例方法 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/consistency/LBStatusConsistencyService.java:29
    • 建议:将 SCHEDULED_EXECUTOR_SERVICE 改为实例字段(非 static),在构造函数中初始化。这样每个 Spring context 有独立的 executor,@PreDestroy 可以安全关闭。
  • MasterEngineSynchronizer 构造函数中 Long.parseLong 对非法环境变量无防护 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/synchronizer/MasterEngineSynchronizer.java:63
    • 建议:添加 NumberFormatException 处理或使用 Optional + 默认值模式。例如:try { Long.parseLong(...) } catch (NumberFormatException e) { Logger.warn(...); defaultValue; }
  • BatchItem.hashCode()/equals() 包含 volatile mutable 字段 sortKey @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/BatchItem.java:115
    • 建议:从 equals()/hashCode() 中移除 sortKey 字段。使用 requestId() 或 (ctx, future) 作为身份标识即可,sortKey 仅用于排序。
  • DefaultBatchDispatcher.shutdownNow() 丢弃排队任务导致 future 未完成 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/DefaultBatchDispatcher.java:69
    • 建议:在 shutdown() 中获取 shutdownNow() 的返回值,解析其中的 BatchItem 并对每个调用 callback.onFailure(),或改用 graceful shutdown(先 shutdown() 再 awaitTermination)。
  • CostBasedDecodeStrategy.buildServerStatus() 中 reserve() 成功后异常不回滚 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedDecodeStrategy.java:258
    • 建议:在 catch 块中添加 optimalEndpoint.release(requestId) 以回滚资源预留。
  • WorkerBatcher.waitForNonEmpty() take-put 模式短暂移除队头元素 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/WorkerBatcher.java:131
    • 建议:考虑使用 queue.peek() 配合短暂 sleep 循环等待非空,避免修改队列状态;或者文档标注 headSortKey()/queueSize() 读取可能短暂不一致。
  • 新增 isResourceAvailable(WorkerEndpoint) 覆盖方法缺少单测 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/resource/DecodeResourceMeasureTest.java:41
    • 建议:增加测试用例:用 WorkerEndpoint 类型参数(实际传入 DecodeEndpoint 实例)调用 isResourceAvailable,验证 instanceof 分发正确;再传入非 DecodeEndpoint 类型的 WorkerEndpoint 验证 fallback 到 super 逻辑。PrefillResourceMeasureTest 同理。
  • FixedWindowBatcherAlgorithm 新增 backpressure gate 缺少测试 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/FixedWindowBatcherAlgorithmTest.java:19
    • 建议:在 FixedWindowBatcherAlgorithmTest 中增加 processQueue 测试:构造 mock BatcherContext 使 getFlexlbBatchFixedMaxInflightBatches() 返回 1,prefillEp().getInflightBatchCount() 返回 1,验证 processQueue 不调用 dispatch 即返回。
  • isNonBatchPath 语义变更(DIRECT → !BATCH)缺少测试 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/strategy/CostBasedPrefillStrategyTest.java:280
    • 建议:增加测试用例:分别设置 BalanceContext.scheduleMode 为 AUTO、DIRECT、BATCH,验证 AUTO 和 DIRECT 走 non-batch path(执行 inflight reservation),BATCH 不走 non-batch path。
  • DefaultRouterTest.setUp() line 95 用 vitLoadBalancer 覆盖 COST_BASED_PREFILL 注册 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/DefaultRouterTest.java:95
    • 建议:如果 VIT 需要独立 strategy enum,应使用正确的 enum 值注册;否则把 93-96 行的全局 register 调用全部移除(反射覆写已足够)。
  • FixedWindowBatcherAlgorithmTest 时间断言容差仅 5ms,CI 易 flaky @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/FixedWindowBatcherAlgorithmTest.java:36
    • 建议:放宽容差到 ±20ms(assertTrue(waitMs >= 80 && waitMs <= 110)),或改用受控时钟来消除时间依赖。
  • FlexlbBatchSchedulerTest.batcher_rejects_when_queue_full 缺少错误码断言 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/FlexlbBatchSchedulerTest.java:346
    • 建议:添加 assertEquals(StrategyErrorType.QUEUE_FULL.getErrorCode(), response.getCode()) 确保拒绝原因正确。
  • FlexlbBatchSchedulerTest.submit_rejects_when_global_inflight_limit_reached 第一个 future 未被 await 或断言 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/FlexlbBatchSchedulerTest.java:324
    • 建议:将返回值赋给变量,在 releaseBlock.countDown() 后 await 并断言 isSuccess(),确保第一个请求正常完成。
  • DefaultRouterTest.should_log_error_when_master_request_is_null 断言过弱 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/DefaultRouterTest.java:353
    • 建议:补充 assertFalse(response.isSuccess()) 并验证错误码,确保 null request 被正确拒绝。
  • RouteServiceTest 使用 .block() 无超时,响应式管道挂起将导致测试无限阻塞 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/service/RouteServiceTest.java:61
    • 建议:改用 .block(Duration.ofSeconds(5)) 或 StepVerifier.create(...).expectNext(...).verifyComplete() 加超时保护。
  • DefaultRouterTest 反射覆写私有字段 + fail() 吞异常堆栈 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/DefaultRouterTest.java:134
    • 建议:改为 fail('Failed to mock LoadBalanceStrategyFactory', e) 保留异常链,或考虑通过构造函数注入 loadBalancerMap 避免反射。
  • GrpcWorkerStatusRunner consecutiveFailures 迁移到 WorkerStatus 后缺少失败路径测试 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/sync/runner/GrpcWorkerStatusCheckRunnerTest.java:30
    • 建议:新增测试:mock engineGrpcService 抛异常,验证 workerStatus.getConsecutiveFailures() 递增;连续失败 >= 3 次后验证 endpoint 被移除或 worker 被标记不可用。
  • 用户配置的线程池大小无上限校验 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:808
    • 建议:极端大值(如 --prefill_enqueue_pool_size 1000000)会尝试创建百万线程,在 pool->start() 失败时通过 RTP_LLM_CHECK_WITH_INFO 崩溃,错误信息不明确。建议加上限 clamp(如 max 1024)或在 C++ 侧打印 WARNING 并 clamp 到合理范围,提供清晰的运维反馈。
  • RoleType 字符串验证使用 getattr 会抛 AttributeError 而非 ValueError @ rtp_llm/config/generate_config.py:60
    • 建议:用 try/except AttributeError 包裹 getattr 调用,或检查 hasattr 后再取值,保持和其他分支一致抛 ValueError。不过其他地方 (worker_status.py:82) 也用了同样的 getattr 模式,所以这是项目惯例。
  • decode_step_begin 日志在 model forward 之后才打印 @ rtp_llm/cpp/normal_engine/NormalExecutor.cc:339
    • 建议:如果 decode_step_begin 意图记录 decode 步骤开始,应该移到 model_->forward() 调用之前(与 prefill_batch_begin 对称)。目前位置在 forward 之后,日志时间不能反映 decode 开始时间。
  • publishNormalDeviceState 每个 stream 触发 O(B) 个 CUDA kernel launch @ rtp_llm/cpp/normal_engine/NormalExecutor.cc:724
    • 建议:对 B 个 decode streams,可以用一次 batched torch::arange 或 torch::add(seq_lens_batched, 1) 替代 B 个独立标量 CUDA kernel launch。在 batch=256 时,per-stream add+to 意味着 512 次无用的 kernel dispatch overhead(约数 us/次,累计可达百 us 级)。考虑将所有 stream 的 seq_len 组成一个 [B] 形状 tensor 做一次 add。
  • prepareGrpcNormalDeviceState 为每个 pending stream 做 torch::full CUDA 分配 @ rtp_llm/cpp/normal_engine/NormalExecutor.cc:639
    • 建议:每个 pending gRPC stream 首次 decode 时分配 2 个 scalar CUDA tensor。对于 PD decode 角色批量接收大量 stream 时,大量小 tensor 分配会碎片化 CUDA allocator。考虑复用 pre-allocated 的 scalar tensor 或使用 pinned_host+async copy。
  • RpcCpuTpBroadcaster::broadcast 在每次调用时复制 peer_tp_ranks_ 和 group_key_ @ rtp_llm/cpp/distribute/RpcCpuTpBroadcaster.cc:167
    • 建议:broadcast() 在热路径中每次调用都复制 peer_tp_ranks_ vector 和 group_key_ string。这些值在 initialize() 后不变。可以在 initialize() 后使用 shared_ptr<const vector> 或直接用 const 引用(initialized 后不会 reset),避免每次 broadcast 的堆分配。
  • dispatchOutputAsync 中 allStreams() 被多次调用,每次创建新 list @ rtp_llm/cpp/normal_engine/NormalExecutor.cc:748
    • 建议:allStreams() 每次调用都复制 decode_streams_ 并拼接 context_streams_,涉及 shared_ptr 引用计数。在 dispatchOutputAsync 路径中,stream_groups_copy 已经是值拷贝,allStreams() 再多调一次是浪费。建议只调一次并传递引用/const ref,或让 StreamGroups 缓存 allStreams 结果。
  • asyncDebugEnabled() 每次调用都 getenv + 构造 std::string @ rtp_llm/cpp/normal_engine/NormalOutputDispatcher.cc:17
    • 建议:虽然只在 error path 调用,但 getenv + string 构造比 static bool 缓存模式多出不必要的开销。其他 env flag(如 useStreamAsync)已经用了 static const bool + lambda 一次性读取的模式,建议统一。
  • request_arrival 日志 prompt_len 对非 prompt 请求(如 chat/messages)始终为 0 @ rtp_llm/frontend/frontend_server.py:265
    • 建议:对于 OpenAI Chat 格式请求,prompt 字段不存在。建议同时检查 messages 字段长度,或使用更通用的 input_len 计算。
  • flexlbBatchQueueMaxSize 从 64 大幅提高到 1024,无内存/延迟告警 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/FlexlbConfig.java:359
    • 建议:16 倍增加默认队列深度。在高并发场景下可能导致请求堆积、内存增长和尾延迟恶化。建议添加日志或指标监控队列深度,或在文档中明确说明调整原因和预期影响。
  • FixedWindowBatcherAlgorithm backpressure 使用 1ms sleep 可能导致 CPU 空转 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/FixedWindowBatcherAlgorithm.java:80
    • 建议:持续高压时 1ms sleep 循环等同于忙等。建议使用 exponential backoff(如从 1ms 增到 10ms),或使用 CountDownLatch/Condition 通知机制,避免 CPU 浪费。
  • getParsedSloBuckets 存在 TOCTOU 竞态:并发 setCostSloBuckets 可导致写入过期的解析结果 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/FlexlbConfig.java:478
    • 建议:costSloBuckets 非 volatile,setCostSloBuckets 和 getParsedSloBuckets 之间存在竞态窗口:解析线程读取旧 costSloBuckets 后,setCostSloBuckets 置空 parsedSloBuckets,解析线程再写回过期结果。如需线程安全,可将解析逻辑放在 setCostSloBuckets 中(eager parsing),或对两个字段加 synchronized。
  • parseValue 拼接 JSON 字符串未转义,含特殊字符的枚举值会导致解析异常 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/ConfigService.java:197
    • 建议:若 value 含引号或反斜杠,拼接出的 JSON 不合法。建议使用 Jackson ObjectMapper.readValue(value, targetType) 或先用 fromName/valueOf 直接匹配枚举名,而非构造 JSON 字符串。当前由外层 try-catch 兜底不会崩溃,但错误信息难以定位。
  • Logger.logbackLogger() 硬转型 logback Logger,非 logback 环境下 ClassCastException @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/util/Logger.java:83
    • 建议:如果测试或其他环境使用非 logback 的 SLF4J 绑定(如 SimpleLogger),此处会抛 ClassCastException。可加 instanceof 检查并优雅降级,或在文档中明确标注 logback 为必须依赖。
  • setPrefillCoefficients 静默吞掉 NumberFormatException,无日志输出 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/FlexlbConfig.java:340
    • 建议:catch 块内添加 log.warn("Failed to parse prefill coefficients: {}", csv, e),否则运维无法感知配置解析失败,排查代价模型异常时缺少关键线索。
  • applyEnvironmentOverrides 通过反射直接设置字段,绕过自定义 setter 的副作用 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/ConfigService.java:86
    • 建议:改用 setter 方法调用(通过 PropertyDescriptor 或显式构造 setter method name),或在 isSupportedType 中排除有自定义 setter 的字段(如 costSloBuckets)。当前通过 COST_SLO_BUCKETS 环境变量设置值时不会触发 parsedSloBuckets 缓存失效。
  • ExpirationCleaner 清理过期 worker 未同步清理 EndpointRegistry 中的 endpoint @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/schedule/ExpirationCleaner.java:36
    • 建议:在 doClean 中删除 worker 后, 通过 EndpointRegistry 回调关闭对应的 endpoint (endpoint.close()), 或让 EndpointRegistry 订阅 worker 状态变更事件.
  • PrefillEndpointTest 缺少 @AfterEach 关闭 batcher 线程 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/endpoint/PrefillEndpointTest.java:35
    • 建议:添加 @AfterEach void tearDown() { if (endpoint != null) endpoint.close(); }

Checklist ✅ (56 items passed)

Strengths

  • 在 FIFOScheduler 的 activated/finished 关键路径添加了结构化 access log,覆盖 prefill/decode 角色区分,对生产排障和全链路追踪有显著价值
  • streamLogTag 从 .cc 实现移到 .h 内联,减少了虚函数调用开销,适合日志热路径
  • PD 分离线程池大小参数化(prefill_enqueue/worker_lambda/slot_pool_size),支持运维动态调优而非硬编码
  • 线程池参数默认值 0 回退到公式计算,负值也安全回退,避免配置错误导致崩溃
  • 修复了 streamLogTag() 的 ODR 违规(先加 inline 再删 .cc 中的重复定义)
  • backpressure gate 默认关闭 (default 0),保持了 fixed_window 原有的 always-dispatch 行为,向后兼容
  • 将原来的 getFlexlbBatchMaxInflightBatchesPerWorker 拆分为 slo/fixed_window 独立配置,避免两种算法互相干扰

@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/0 · P1/12 · P2/22 · P3/3

Blocking Issues

P1

  • w4a8_int4_per_channel_quant_weight.py 缺少 import logging,NameError 掩盖 ImportError @ rtp_llm/model_loader/w4a8_int4_per_channel_quant_weight.py:31
    • 建议:在文件顶部添加 import logging。
  • cutlass_moe.py get_cutlass_batched_moe_mm_data 为 None 时无守卫直接调用 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/cutlass_moe.py:455
    • 建议:在 check_conditions 中添加 checker.check(get_cutlass_batched_moe_mm_data is not None)。
  • cutlass_w4a8_moe.py compute_reorder_stride/w4a8_group_gemm_ptpc 为 None 时 init 直接崩溃 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/cutlass_w4a8_moe.py:95
    • 建议:在 check_conditions 中检查 compute_reorder_stride is not None and w4a8_group_gemm_ptpc is not None。
  • PDSepConfig pickle 向后不兼容:旧 tuple 无法反序列化 @ rtp_llm/cpp/pybind/ConfigInit.cc:1756
    • 建议:使用 t.size() >= N 渐进解析,缺失字段填默认值(同 KVCacheConfig 做法)。
  • FlexlbGrpcForwarder: channels.remove 后未 shutdown channel,TCP 连接泄漏 @ rtp_llm/flexlb/flexlb-api/src/main/java/org/flexlb/httpserver/FlexlbGrpcForwarder.java:63
    • 建议:remove 前获取引用并 shutdownNow:ManagedChannel ch = channels.remove(key); if (ch != null) ch.shutdownNow();
  • master_client_test _CaptureMasterClient 签名与 gRPC 实际接口不匹配,全部测试必定失败 @ rtp_llm/server/test/master_client_test.py:45
    • 建议:重写签名为 (addr, request_pb, timeout_s, request_id),断言改为 proto 属性访问。
  • backend_rpc_server_visitor_test _FakeMasterClient 签名缺少参数且参数名不匹配 @ rtp_llm/server/test/backend_rpc_server_visitor_test.py:53
    • 建议:签名改为 (block_cache_keys, cache_key_block_size, input, request_id, input_pb=None),断言 key 改为 input_pb。visitor 补设 _page_rr_route_cache_keys=False。
  • PrefillEndpointTest.calibrateHandlesTaskWithNoBatchId 断言错误,测试必定失败 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/endpoint/PrefillEndpointTest.java:176
    • 建议:将 commitBatch batchId 改为与 requestId 不同的值(如 999L),使 remove(1L) 不命中。
  • EngineSyncRunner.ensureEndpoint() 对 VIT/PDFUSION 抛异常导致整个角色同步中断 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/EngineSyncRunner.java:238
    • 建议:VIT/PDFUSION 不需要 endpoint,ensureEndpoint 应跳过而非抛异常。移除 else throw 或在循环内 try-catch 隔离。
  • detached 线程捕获 this 指针,析构时 use-after-free 风险 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:1863
    • 建议:改用 shared_from_this() 捕获,或在析构中通过 atomic flag + 等待确保 detached 线程退出。
  • completeCancelled 绕过 rollbackOnce 导致 decode slot 双重释放 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/FlexlbBatchScheduler.java:373
    • 建议:在 entry==null 分支也使用 AtomicBoolean 保护 rollback,或在 BatchItem 上添加 rollbackOnce 标志。
  • cacheVersion long→int 截断导致版本号溢出后缓存更新永久阻塞 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/service/grpc/EngineGrpcService.java:91
    • 建议:溢出前 clamp 到 Integer.MAX_VALUE,或将 proto 字段升级为 int64。

Non-blocking Suggestions

P2

  • setPrefillCoefficients 静默吞掉 NumberFormatException,无日志 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/FlexlbConfig.java:340
    • 建议:添加 log.warn("Failed to parse prefill coefficients: {}", csv, e)。
  • Logger 硬耦合 logback,非 logback 后端 ClassCastException @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/util/Logger.java:83
    • 建议:添加 instanceof 检查,非 logback 后端降级为 no-op。
  • GenerateConfig 删除 force_batch 字段可能导致旧请求反序列化失败 @ rtp_llm/config/generate_config.py:179
    • 建议:配置 extra='ignore' 或保留 force_batch 为 deprecated alias。
  • fused_rope_kvcache_op.py prefill/decode 函数 None 时无守卫直接调用 @ rtp_llm/ops/fused_rope_kvcache_op.py:98
    • 建议:在 init 或 prepare 中 assert 非 None,或在模块级 import 失败时直接 raise。
  • cache_key_block_size 参数传入但未使用,成为死参数 @ rtp_llm/server/master_client.py:200
    • 建议:如不需要则移除参数;如需要则在 proto 中添加对应字段。
  • sortedCacheDirty 在 offer 路径不被标脏,新请求可能被 pick 遗漏 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/WorkerBatcher.java:166
    • 建议:在 offer() 中 queue.add 后调用 ctx.markDirty(),确保新入队元素可见。
  • onOfferFailure error 可为 null 但实现未做空指针防护 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/FlexlbBatchScheduler.java:251
    • 建议:加 error != null ? error.getMessage() : "queue full" 防护,或修正接口文档。
  • CostBasedPrefillStrategy 计算了 cacheHit 但未纳入评分,cache-aware 选择失效 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedPrefillStrategy.java:97
    • 建议:将 cacheHit 纳入 computeScore,使 cache-aware routing 真正生效。
  • WorkerBatcher.waitForNonEmpty take/put 模式在 shutdown 时可丢失请求 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/WorkerBatcher.java:131
    • 建议:改用 queue.peek()+LockSupport.park 避免出队,或在 InterruptedException catch 中 drain 剩余。
  • ResponseBufferWriter 队列满时静默丢弃消息,无日志/指标 @ rtp_llm/cpp/model_rpc/ResponseBuffer.cc:124
    • 建议:添加 drop counter 或 log warning,便于排查不完整输出问题。
  • canAdmitUnit 对 group 跳过 token 预算检查 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:200
    • 建议:对 group 也做 max_batch_tokens_size_ 保护,或添加注释说明上游已校验。
  • LBStatusConsistencyService static final 线程池 shutdown 后不可恢复 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/consistency/LBStatusConsistencyService.java:29
    • 建议:改为实例字段,在构造函数中创建、@PreDestroy 中关闭。
  • RpcCpuTpBroadcaster inbox_ 无大小上限,异常情况可能 OOM @ rtp_llm/cpp/distribute/RpcCpuTpBroadcaster.cc:281
    • 建议:增加 inbox_ 大小上限检查(如 1000),超限时 log 警告。
  • applyEnvironmentOverrides 通过 field.set 绕过自定义 setter @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/ConfigService.java:89
    • 建议:对有自定义 setter 的字段调用 setter 而非 field.set。
  • CostBasedPrefillStrategy 每个 endpoint 重复调用 realWaitTimeMs() 3-4 次 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedPrefillStrategy.java:142
    • 建议:在 select() 入口缓存每个 endpoint 的 realWaitTimeMs() 值,后续复用。
  • DecodeEndpoint.inflightKvReserved() 每次调用遍历 ConcurrentHashMap @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/endpoint/DecodeEndpoint.java:97
    • 建议:维护 AtomicLong inflightKvReservedSum,在 reserve()/release()/calibrate() 时增减。
  • InflightEvictor onEvict 回调导致 O(B²) 复杂度 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/endpoint/PrefillEndpoint.java:41
    • 建议:将 onEvict 回调改为空操作,仅在 evictExpiredBatches() 结束后调一次 refresh。
  • EnqueueBatch 热路径同步调用 gc(),持锁遍历所有 entries @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:1601
    • 建议:移除热路径 gc() 调用,依赖后台 GC 线程;或仅在 registry size 超阈值时触发。
  • DefaultBatchDispatcherTest PrefillEndpoint WorkerBatcher 线程未关闭 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/DefaultBatchDispatcherTest.java:240
    • 建议:添加 @AfterEach 调用 PrefillEndpoint.close() 和 dispatcher.shutdown()。
  • 多个 Strategy 测试类使用静态 MODEL_ROLE_WORKER_STATUS 但部分无 @beforeeach clear @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/strategy/CostBasedDecodeStrategyTest.java:88
    • 建议:在所有使用 MODEL_ROLE_WORKER_STATUS 的测试类 @beforeeach 中统一 clear() 所有静态 map。
  • DefaultRouterTest 静态工厂重复注册导致状态污染 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/DefaultRouterTest.java:95
    • 建议:为 VIT 使用独立策略枚举值,或在 @AfterEach 中重置 LoadBalanceStrategyFactory。
  • BatchInflight.lastSeenMs 的 read-compare-write 非原子更新 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/endpoint/BatchInflight.java:76
    • 建议:改用 AtomicLong.updateAndGet(old -> Math.max(old, statusMs)),与 progressBaseMs 处理保持一致。

P3

  • RtpLLMOp.cc 重复 include autil/EnvUtil.h @ rtp_llm/cpp/pybind/multi_gpu_gpt/RtpLLMOp.cc:1
    • 建议:删除重复的 #include。
  • PoolMetrics active/queued/completed 成员从未递增 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.h:410
    • 建议:在 pushTask 前后维护计数器,或移除未使用字段。
  • ipPort.split(":") 不支持 IPv6 地址 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/GrpcWorkerStatusRunner.java:54
    • 建议:使用 lastIndexOf(':') 或统一 host:port 解析工具函数。

Checklist ✅ (56 items passed)

Strengths

  • WorkerStatus 重构移除 ConcurrentHashMap localTaskMap 及其 CAS 操作,简化为引擎上报权威值,降低热路径 CPU 开销并减少状态管理 bug
  • ConfigService 新增 try-catch 防止配置解析失败导致服务无法启动,提升鲁棒性
  • RoleType/LoadBalanceStrategyEnum 添加 @JsonCreator/@jsonvalue 支持多种格式反序列化
  • Logger 重构为直接委托 SLF4J/logback,消除自定义 globalLogLevel 双重过滤
  • 对 rtp_kernel 依赖做了 try/except 包裹,使模块在缺少 kernel 时仍能被 import

…ster routing

Add MasterConfig.disable_domain_fallback field with env var
MASTER_DISABLE_DOMAIN_FALLBACK to prevent fallback to VipServer
domain routing when master is unavailable or not configured.
When enabled, requests will fail with ROUTE_ERROR instead of
silently degrading to domain-based service discovery.
@LLLLKKKK

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/1 · P1/15 · P2/31 · P3/2

Blocking Issues

P0

  • RpcCpuTpBroadcaster 使用 DP 过滤后地址列表,DP>1 时越界崩溃 @ rtp_llm/cpp/normal_engine/NormalExecutor.cc:115
    • 建议:改用 params.runtime_config.all_worker_grpc_addrs,按 world_rank 索引,大小为 world_size。

P1

  • cutlass_moe.py 导入失败设为 None,运行时 TypeError 崩溃 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/cutlass_moe.py:455
    • 建议:在 check_conditions 或 init 中检查 is None 并抛出清晰异常,或保持原始 raise 而非设为 None。
  • setPrefillCoefficients 解析失败时系数部分更新不一致 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/FlexlbConfig.java:326
    • 建议:先解析到临时变量,全部成功后再赋值给实例字段。catch 块添加 warn 日志。
  • RoleType.fromString 遇未知值抛异常,导致整条 WorkerStatusResponse 反序列化失败 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/dao/route/RoleType.java:50
    • 建议:用 try-catch 包裹 valueOf,未知值返回 null(与 value==null 一致)。
  • finish_lambda 双重 std::move 导致使用已移动对象 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:1860
    • 建议:确认 pushTask 返回 ERROR 时 lambda 是否已被消费。若已消费需在 pushTask 前保留副本。
  • waitStreamBeforeRun 使用 static 局部变量捕获成员配置 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:877
    • 建议:改为普通局部变量 int64_t,避免多实例共享和溢出。
  • getEngineScheduleInfo 在 shared_lock 下写入 entry.task_info.phase,数据竞争 @ rtp_llm/cpp/model_rpc/RpcServerRuntimeMeta.h:33
    • 建议:将 derivePhase 结果写入本地 copy:auto task_copy = entry.task_info; task_copy.phase = derivePhase(...);
  • PDSepConfig pickle 反序列化不兼容旧格式 @ rtp_llm/cpp/pybind/ConfigInit.cc:1756
    • 建议:改为 if (t.size() < 20) 并对位置 20-25 用条件解析:t.size() > idx ? t[idx].cast<...>() : default_value。
  • FlexlbGrpcForwarder channel 错误时 remove 未 shutdown,gRPC 资源泄漏 @ rtp_llm/flexlb/flexlb-api/src/main/java/org/flexlb/httpserver/FlexlbGrpcForwarder.java:63
    • 建议:ManagedChannel removed = channels.remove(channelKey); if (removed != null) removed.shutdownNow();
  • w4a8 量化文件缺少 logging import,ImportError 路径 NameError @ rtp_llm/model_loader/w4a8_int4_per_channel_quant_weight.py:31
    • 建议:在文件头部添加 import logging。
  • ExpirationCleaner 会立即驱逐新创建的 WorkerStatus @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/schedule/ExpirationCleaner.java:45
    • 建议:跳过 statusLastUpdateTime < 0 的条目,或初始化为当前时间。
  • getOrCreateWorkerStatus 非原子操作导致 Endpoint 持有孤儿 WorkerStatus @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/EngineSyncRunner.java:204
    • 建议:改为 workerStatuses.computeIfAbsent(key, k -> new WorkerStatus(...));
  • DefaultBatchDispatcher 回调循环无 per-item 错误隔离 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/DefaultBatchDispatcher.java:128
    • 建议:对每个 callback 调用包裹 try-catch,异常时 log+continue。
  • FlexlbBatchScheduler.submit() 异常路径未 rollback decode KV 预留 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/FlexlbBatchScheduler.java:137
    • 建议:在 catch 块增加 if (routeResponse != null) rollback(routeResponse)。
  • EngineSyncRunner.ensureEndpoint() 对 VIT/PDFUSION 角色抛异常终止整轮同步 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/EngineSyncRunner.java:239
    • 建议:改为 log.warn + return,跳过不支持的角色类型。
  • server_config_setup.py DeepEP 日志两行合并为一行 @ rtp_llm/config/server_config_setup.py:109
    • 建议:在 USE_DEEPEP_LOW_LATENCY 行末添加 \n。

Non-blocking Suggestions

P2

  • fused_rope_kvcache_op ImportError 后函数为 None,调用时 TypeError @ rtp_llm/ops/fused_rope_kvcache_op.py:53
    • 建议:在 init 中检查关键函数 is None 时 raise ImportError。
  • cutlass_w4a8_moe compute_reorder_stride=None 在 init 中无保护直接调用 @ rtp_llm/models_py/modules/factory/fused_moe/impl/cuda/executors/cutlass_w4a8_moe.py:95
    • 建议:在 check_conditions 中检查 compute_reorder_stride is not None。
  • master_group_args --master_disable_domain_fallback 使用 type=bool 解析异常 @ rtp_llm/server/server_args/master_group_args.py:50
    • 建议:改为 action='store_true' 或使用自定义 str2bool 函数。
  • is_backend_service_ready 硬编码返回 True,健康检查失去意义 @ rtp_llm/server/backend_rpc_server_visitor.py:555
    • 建议:考虑检查 FlexLB master 连通性,或在 disable_domain_fallback=True 时才无条件返回 True。
  • MasterClient gRPC channel 无主动回收,地址变更后旧 channel 泄漏 @ rtp_llm/server/master_client.py:133
    • 建议:在 _get_channel 中添加简单 LRU 或检查 target 是否与当前 master_addr 一致。
  • trans_input 在每次请求中被重复调用两次 @ rtp_llm/server/backend_rpc_server_visitor.py:219
    • 建议:缓存 trans_input 结果,或在 enqueued_by_master=True 时跳过完整 trans_input。
  • 测试 mock 签名与实际 gRPC 接口完全不匹配 @ rtp_llm/server/test/master_client_test.py:46
    • 建议:重写测试 mock 匹配新的 gRPC/protobuf 接口签名。
  • DecodeRpcServer::initThreadPool 条件检查逻辑反转 @ rtp_llm/cpp/model_rpc/DecodeRpcServer.cc:74
    • 建议:将条件改为 if (resource_.workers.empty()) { return; }。
  • EnqueueBatch 热路径尾部调用 gc() 持全局锁遍历 map @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:1604
    • 建议:移除此处同步 GC,依赖后台 GC 线程。
  • ResponseBufferWriter::Write 队列满时静默丢弃旧数据 @ rtp_llm/cpp/model_rpc/ResponseBuffer.cc:125
    • 建议:至少添加 WARNING 日志记录被丢弃的 response。
  • RpcServerRuntimeMeta::trimFinishedStreams 定义但从未调用 @ rtp_llm/cpp/model_rpc/RpcServerRuntimeMeta.h:132
    • 建议:在适当位置调用 trimFinishedStreams(),或删除死代码。
  • PoolMetrics 的 active/queued/completed 始终为 0 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.h:19
    • 建议:在 pushTask/async 前后递增/递减 active/queued/completed。
  • admitWaitingUnits 移除错误 unit 时未调用 finish() @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:218
    • 建议:在 erase 前对 unit 中每个 stream 调用 finish()。
  • canAdmitUnit 对 group unit 跳过 token 预算检查 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:200
    • 建议:若 group 由 FlexLB 保证预算则添加注释说明假设,否则也做 token 预算校验。
  • RpcCpuTpBroadcaster inbox 无界增长风险 @ rtp_llm/cpp/distribute/RpcCpuTpBroadcaster.cc:281
    • 建议:对 inbox_ 添加 TTL 清理或大小限制。
  • get_backend_role_addrs 的 cache_key_block_size 参数为死代码 @ rtp_llm/server/master_client.py:200
    • 建议:若为预留字段则传入 PB;若不需要则从签名移除。
  • RoleAddr.validate_role 对无效字符串抛 AttributeError 而非 ValueError @ rtp_llm/config/generate_config.py:60
    • 建议:改为 getattr(..., None) 并在 None 时 raise ValueError。
  • Logger 硬转型 logback Logger,非 logback 环境 ClassCastException @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/util/Logger.java:83
    • 建议:用 instanceof 检查,不匹配时降级为 no-op。
  • getParsedSloBuckets 静默忽略格式错误的 SLO bucket 条目 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/FlexlbConfig.java:488
    • 建议:在 catch 块添加 warn 日志。
  • updateFromResponse 多字段非原子写入无锁保护 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/dao/master/WorkerStatus.java:54
    • 建议:在 updateFromResponse 中使用 lock,或将频繁读取字段标为 volatile。
  • FlexlbGrpcForwarder channels computeIfAbsent + remove 存在竞态 @ rtp_llm/flexlb/flexlb-api/src/main/java/org/flexlb/httpserver/FlexlbGrpcForwarder.java:54
    • 建议:使用 channels.compute() 原子操作。
  • CostBasedDecodeStrategy.weightedRandomSelection 浮点精度可能跳过最后候选 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedDecodeStrategy.java:239
    • 建议:循环后直接返回最后一个元素。
  • BatchItem.hashCode() 包含可变 sortKey 字段 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/BatchItem.java:115
    • 建议:从 equals/hashCode 中移除 sortKey。
  • LBStatusConsistencyService static final executor shutdown 后无法重建 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/consistency/LBStatusConsistencyService.java:29
    • 建议:改为实例字段而非 static final。
  • 静态线程池在构造函数中赋值,多次实例化泄漏且核心线程数过大 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/synchronizer/MasterEngineSynchronizer.java:170
    • 建议:改为 static initializer 确保单例,并根据 worker 数量动态计算核心线程数。
  • SloBudgetBatcherAlgorithm.pickWithinIncrementalBudget 中 O(n²) 列表分配 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/SloBudgetBatcherAlgorithm.java:166
    • 建议:直接在 picked 上 add/remove 原地试探,或让 predictor 支持增量计算。
  • CostBasedDecodeStrategy realKvUsed() 单次路由决策中重复调用 3-4 次 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedDecodeStrategy.java:135
    • 建议:在 select() 入口缓存每个 endpoint 的 realKvUsed 值。
  • CostBasedPrefillStrategy realWaitTimeMs() 单次决策中重复调用约 4N 次 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedPrefillStrategy.java:142
    • 建议:在 doSelect() 开头缓存每个 endpoint 的值。
  • FlexlbBatchScheduler.flushItems() ArrayList.contains() 导致 O(n²) @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/FlexlbBatchScheduler.java:276
    • 建议:改用 HashSet 实现 O(1) 查找。
  • FixedWindowBatcherAlgorithmTest 时间容差窗口过窄,flaky 风险 @ rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/FixedWindowBatcherAlgorithmTest.java:36
    • 建议:放宽容差或注入 Clock 接口消除壁钟依赖。
  • RtpLLMOp.cc 重复 include autil/EnvUtil.h @ rtp_llm/cpp/pybind/multi_gpu_gpt/RtpLLMOp.cc:9
    • 建议:删除第 9 行的重复 #include。

P3

  • GatherBatchScheduler 重复实现 gatherCountStreams/gatherFlattenRunning @ rtp_llm/cpp/engine_base/schedulers/GatherBatchScheduler.h:144
    • 建议:将 countStreams 改为 protected,删除重复实现。
  • GenerateStream::prepare() 中 wait_time_us_ 未被设置 @ rtp_llm/cpp/engine_base/stream/GenerateStream.cc:619
    • 建议:在 prepare() 的 lock guard 内设置 wait_time_us_。

Checklist Violations (6 fail / 56 total)

General Principles Checklist

  • [6.1] Software Engineering — DRY:重复非平凡逻辑被抽取或显式复用 → issue GatherBatchScheduler 重复实现 gatherCountStreams/gatherFlattenRunning
    GatherBatchScheduler 重复实现了 FIFOScheduler::countStreams。影响较小。
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue FlexlbBatchScheduler.submit() 异常路径未 rollback decode KV 预留
    FlexlbBatchScheduler.submit() 异常路径未 rollback decode KV 预留。
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue setPrefillCoefficients 解析失败时系数部分更新不一致
    多处静默吞异常:getParsedSloBuckets、setPrefillCoefficients catch 无日志。
  • [6.1] Architecture — 可观测性:日志/指标/超时可操作、非噪声 → issue PoolMetrics 的 active/queued/completed 始终为 0
    PoolMetrics active/queued/completed 始终为 0,ResponseBuffer 丢弃数据无日志。
  • [6.1] Architecture — 兼容性:公开 API/持久数据/配置/环境迁移安全 → issue PDSepConfig pickle 反序列化不兼容旧格式
    PDSepConfig pickle 新格式(26元素)不兼容旧格式(20元素),滚动升级时中断。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue 测试 mock 签名与实际 gRPC 接口完全不匹配
    FlexLB 新增 Java 模块有单测,但 EngineSyncRunnerTest 和 GrpcWorkerStatusCheckRunnerTest 缺少异常路径覆盖。

Strengths

  • HTTP→gRPC 迁移整体设计合理:channel 缓存、error 处理、slave fallback 逻辑完整
  • ServerStatus.validate_role 新增 int/RoleType/str 三种类型支持,兼容 gRPC proto int enum 和旧 JSON str
  • ConfigService 对 FLEXLB_CONFIG 和 TrafficPolicy 解析都加了 try-catch 兜底,解析失败使用默认配置
  • WorkerStatus 简化为 updateFromResponse 一次性更新引擎权威值,大幅降低并发竞态风险
  • MasterClient 从 HTTP/aiohttp 迁移到 gRPC 减少序列化开销,是合理的架构演进
  • health_check_path 参数化设计良好,支持自定义健康检查路径并做了边界处理

wzy-99 added 4 commits June 29, 2026 23:00
… configurable default schedule mode

1. CostBasedPrefillStrategy.isNonBatchPath(): remove scheduleMode check
   - When flexlbBatchEnabled=true, only check the global flag; schedule mode
     is no longer a condition for creating request-ID placeholder entries.
   - This eliminates ~93% redundant inflightBatches entries that were mixing
     batch-ID and request-ID keys in the same ConcurrentHashMap.

2. PrefillEndpoint.calibrate(): log inflightBatches.size() instead of keySet()
   - Avoid dumping 250+ IDs per calibrate log line when placeholder entries exist.

3. FlexlbConfig: add DEFAULT_SCHEDULE_MODE env var support
   - New field defaultScheduleMode (String, default 'AUTO'), overridable via
     DEFAULT_SCHEDULE_MODE env var (AUTO/BATCH/DIRECT).

4. FlexlbServiceImpl: use config-driven default schedule mode
   - Replace hardcoded toScheduleMode() with resolveScheduleMode() that falls
     back to config.getDefaultScheduleModeEnum() when proto sends default value.
…ng existing dashboard keys

Add BatchSchedulerReporter that reports batcher queue depth, batch wait time,
dispatch reason QPS, scheduler inflight size, and prefill inflight batch count.
All 5 metric keys exactly match existing Grafana dashboard keys so batch-path
data appears on the same panels as non-batch path without dashboard changes.

Key changes:
- BatchSchedulerReporter: 5 metrics (GAUGE/QPS, PRECISE) reusing ROUTING_QUEUE_*,
  ENGINE_BALANCING_MASTER_SELECT_DETAIL, ENGINE_LOCAL_TASK_MAP_SIZE,
  ENGINE_RUNNING_TASK_INFO_SIZE constants
- MetricConstant: removed unused BATCH_* constants (now using existing keys)
- FlexlbBatchScheduler.reportBatchMetrics: scheduler inflight + delegates
  per-worker metrics to PrefillEndpoint.reportBatchMetrics(reporter)
- DefaultBatchDispatcher: cleanup removed gRPC timing metrics, empty
  reportGrpcFailure method, unused @scheduled import; reverted
  ThreadPoolExecutor back to ExecutorService
- PrefillEndpoint: added reportBatchMetrics method (WorkerEndpoint
  state normalization principle)
- Batcher components: wired BatchSchedulerReporter through constructor chain
- Tests: updated constructor calls, 46 tests pass
…build sorted view

The PriorityBlockingQueue iterator returns heap physical order (level-order
traversal), not sorted order — so sortedItems() already needs copy+sort.
The cache added a stale-view bug where WorkerBatcher.offer() adds items
without invalidating the cache, causing processQueue to see incomplete
views and split one logical batch into two dispatches (S->M) within 1-5ms.

Queue size is typically <50 items, copy+sort cost is negligible (<1us)
vs prefill latency (~200-500ms). Dropping the cache fixes the bug and
simplifies the code.
wzy-99 added 5 commits July 1, 2026 18:09
When finishTask() is called after dequeue() has already moved the request
from running_streams_ to finished_streams_, it created a duplicate entry
with default batch_id=-1, polluting FlexLB's GetWorkerStatus reports.

The root cause: finishTask always wrote to finished_streams_ even when
the request was no longer in running_streams_. In FlexLB batch mode,
this happens when prefill completes at remoteLoadCacheEnd (dequeue with
batch_id=X) but remoteGenerate subsequently fails, triggering finishTask
in finish_lambda which creates a second entry with batch_id=-1.

Fix: return early from finishTask when the request is not found in
running_streams_, since it was already reported via dequeue().  The
decode failure is still propagated to FlexLB frontend through
markResponseEntryDone, which triggers retry with a new batch_id.
Upgrade 5 key decode-side log statements to INFO level to enable
request-level lifecycle tracing in production:
- receive request (LocalRpcServer)
- load cache from prefill done (DecodeRpcServer)
- start to local generate (DecodeRpcServer)
- local generate done (LocalRpcServer + DecodeRpcServer)

Previously all decode inference events were DEBUG-only, making it
impossible to trace a request through the decode pipeline at INFO log
level. With this change, each request leaves a clear start-to-end trail
on decode, matching the observability of the prefill side.
…l(20ms) to 5000ms

The previous default fallback of syncRequestTimeoutMs to syncEngineStatusInterval
(20ms) caused ALL GetCacheStatus gRPC calls to fail with DEADLINE_EXCEEDED,
resulting in hit_cache=0 for all requests because WorkerStatus.cacheStatus was
never populated.

Set explicit default of 5000ms (5s) for gRPC calls to engine-side KV cache
status queries, which typically take 600-800ms on L20B ARM architecture.
- StreamCacheResource::reuseCache() no longer requires per-request
  reuse_cache in protobuf; a global switch based on REUSE_CACHE=1
- DefaultBatchDispatcher sets reuse_cache=true & enable_device_cache=true
  as defensive measure for dispatched batch requests
1. GenerateStream: 将 FINISHED 状态置位前移到 releaseResource() 之前,
   修复 Path B 中 tryReleaseKVBlock 因状态时序问题导致 insertIntoCache
   从未被调用、SharedBlockCache items 恒为 0 的 bug

2. DefaultBatchDispatcher: 移除 buildInput 中对 reuse_cache /
   enable_device_cache 的硬编码 true 覆盖,改为从前端 protobuf 继承

3. DecodeResourceMeasure: 日志中 getIp() → ipPort() 提高可读性
@LLLLKKKK

LLLLKKKK commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/0 · P1/4 · P2/8 · P3/1

Blocking Issues

P1

  • FIFOScheduler 移除了 cp_force_single_prefill 和 max_inited_kv_cache_streams 但对应测试未更新 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:28
    • 建议:在 canAdmitUnit 中添加 cp_force_single_prefill 和 max_inited_kv_cache_streams 的检查逻辑,或者如果确实不需要这些限制,更新/删除对应的测试用例使其与实际行为一致。
  • EngineSyncRunner.ensureEndpoint() 对 VIT/PDFUSION 角色抛出未处理异常导致同步循环失效 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/runner/EngineSyncRunner.java:228
    • 建议:为 VIT 和 PDFUSION 角色添加 endpoint 注册逻辑(可复用 PrefillEndpoint 或创建通用 WorkerEndpoint),确保所有受支持角色的同步循环正常运行。
  • FlexlbBatchScheduler.cancel() 在 gRPC handler 线程上执行同步阻塞 gRPC 调用 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/FlexlbBatchScheduler.java:174
    • 建议:将 cancelPrefill() 改为异步执行(提交到 dispatchExecutor 或单独的轻量线程池),避免阻塞 gRPC/Netty 处理线程。
  • backend_rpc_server_visitor_test.py 中 _FakeMasterClient 签名与实际 gRPC 接口不匹配 @ rtp_llm/server/test/backend_rpc_server_visitor_test.py:55
    • 建议:修改 _FakeMasterClient.get_backend_role_addrs 签名为 (self, block_cache_keys, cache_key_block_size, input, request_id, input_pb=None),同步更新 calls 字典 key 和相关断言。

Non-blocking Suggestions

P2

  • canAdmitUnit 对 group 类型跳过 max_batch_tokens_size 限制 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:200
    • 建议:对 group 类型同样执行 token 总量检查,或在注释中明确说明 group 豁免此限制的原因(如上层已做预算验证),并添加边界测试。
  • reuseCache() 移除 per-request 控制导致已有测试失败 @ rtp_llm/cpp/engine_base/stream/StreamCacheResource.cc:605
    • 建议:保留 per-request 的 reuse_cache 控制,或如果确属设计意图,则同步更新 StreamCacheResourceTest.cc 的 testReuseCacheControl 测试,并在 CHANGELOG 中标注此 breaking change。
  • RpcCpuTpBroadcaster::broadcast 对瞬时网络故障使用 CHECK 直接 abort @ rtp_llm/cpp/distribute/RpcCpuTpBroadcaster.cc:154
    • 建议:将 broadcast() 返回类型改为 absl::Status,错误情况下返回错误状态让调用方决定重试或上报,至少对 timeout 和网络错误使用 soft error。
  • streamLogTag() 简化后丢失关键诊断信息 @ rtp_llm/cpp/engine_base/stream/GenerateStream.h:465
    • 建议:至少保留 source_request_id 和 source_role 字段,或提供 streamLogTagFull() 供 access log 使用。
  • FlexlbGrpcForwarder 移除 channel 时未调用 shutdownNow 导致资源泄漏 @ rtp_llm/flexlb/flexlb-api/src/main/java/org/flexlb/httpserver/FlexlbGrpcForwarder.java:63
    • 建议:在 catch 块中对 channels.remove() 的返回值调用 shutdownNow(),或使用 ConcurrentHashMap.compute() 原子化 remove + shutdown。
  • FlexlbConfig.setPrefillCoefficients() 解析失败时静默吞异常导致半更新状态 @ rtp_llm/flexlb/flexlb-common/src/main/java/org/flexlb/config/FlexlbConfig.java:249
    • 建议:catch 块中添加 warn 级别日志记录原始值和异常信息;解析失败时回滚所有字段到默认值(all-or-nothing),避免半更新状态。
  • GenerateConfig 删除 force_batch 字段可能影响下游兼容性 @ rtp_llm/config/generate_config.py:179
    • 建议:在 GenerateConfig 中添加 @Property 提供向后兼容别名(如 force_batch 返回 False 并 warn deprecated),或在 CHANGELOG 中明确标注此 breaking change。
  • is_backend_service_ready 健康检查被绕过,始终返回 True @ rtp_llm/server/backend_rpc_server_visitor.py:555
    • 建议:添加条件判断:FlexLB 模式下绕过旧检查(或实现基于 FlexLB master 可达性的新检查),非 FlexLB 模式保留原逻辑。删除注释掉的死代码。

P3

  • ExceptionType 枚举值排序不单调 @ rtp_llm/config/exceptions.py:86
    • 建议:将 ROUTE_ERROR=8500 移到新增枚举值之前,按数值升序排列。

Checklist Violations (6 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 兼容性:公开 API/持久数据/配置/环境迁移安全 → issue GenerateConfig 删除 force_batch 字段可能影响下游兼容性
    GenerateConfig 删除了 force_batch: bool 和 batch_group_timeout 字段,替换为 group_timeout。GenerateInput 的 batch_group_size/batch_group_id 重命名为 group_size/group_id。直接访问 config.force_batch 的下游代码将抛出 AttributeError,通过旧字段名构造 GenerateInput 的代码将 TypeError。C++ 侧 GenerateConfig.h 的 JSONIZE 也已移除 force_batch,wire format 不兼容。
  • [6.1] Architecture — 可观测性:日志/指标/超时可操作、非噪声 → issue is_backend_service_ready 健康检查被绕过,始终返回 True
    is_backend_service_ready 方法中原有的后端角色可用性检查被完全注释掉,替换为无条件返回 True(注释:force routing through FlexLB)。FlexLB 模式下原检查会 false negative(VipServer 返回空),但无条件 True 在非 FlexLB 模式下也生效,且留下了注释掉的死代码。
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue FlexlbGrpcForwarder 移除 channel 时未调用 shutdownNow 导致资源泄漏
    gRPC 调用失败时 catch 块执行 channels.remove(channelKey) 但丢弃返回值,未对被移除的 ManagedChannel 调用 shutdownNow()。同项目 AbstractGrpcClient.Invoker.shutdown() 展示了正确做法(先 shutdown 再 awaitTermination 再 shutdownNow)。每次 gRPC 失败泄漏一个 channel,持续故障场景下 TCP 连接和 Netty 资源线性累积。
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue FlexlbConfig.setPrefillCoefficients() 解析失败时静默吞异常导致半更新状态
    setPrefillCoefficients() 在 NumberFormatException 时静默 catch。配置 PREFILL_COEFFICIENTS 为 "290,abc,1.21e-8" 时,a0 被更新但 a1/a2 保持默认,产生半更新状态,可能导致严重的预测偏差且无任何日志提示,极难排查。
  • [6.1] Software Engineering — KISS/YAGNI:无投机性抽象 → issue ExceptionType 枚举值排序不单调
    在 route error 分组中,ROUTER_QUEUE_FULL=8502、ROUTER_QUEUE_TIMEOUT=8503、ROUTER_REQUEST_CANCELLED=8504 排在 ROUTE_ERROR=8500 之前,非数值升序排列,增加维护者混淆风险。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue backend_rpc_server_visitor_test.py 中 _FakeMasterClient 签名与实际 gRPC 接口不匹配
    _FakeMasterClient.get_backend_role_addrs 签名为 (self, block_cache_keys, input, request_id, input_pb_bytes=None),而生产代码 MasterClient 签名为 (self, block_cache_keys, cache_key_block_size, input, request_id, input_pb=None)。缺少 cache_key_block_size 必填参数,关键字参数 input_pb_bytes 与实际的 input_pb 不一致。调用方使用关键字参数传递,运行时将抛出 TypeError。测试会在 CI 中失败。

Strengths

  • ScheduleUnit 抽象将调度逻辑从单一 stream 提升到 group 粒度,batch group 调度语义更清晰,新增的 group isolation 测试覆盖全面(group-first、singles-first、interleaved、two-groups 等场景)
  • GenerateStream 生命周期方法(prepare/isReady/activate/advance/finish)替代 moveToNext() 状态机,职责划分更明确,消除了状态机驱动的间接性
  • RpcCpuTpBroadcaster 使用 inbox/CV 模式实现跨节点 CPU TP 广播,设计简洁,验证充分
  • ResponseBuffer 为 PD 分离场景提供了 producer-consumer 解耦机制,kMaxQueueSize 防止 OOM
  • FlexLB 架构设计清晰:WorkerEndpoint 抽象 + BatcherAlgorithm 策略模式 + InflightEvictor 泛型复用
  • ZooKeeper master 选举通知 bug 修复(!localIp.equals 反转)是关键的正确性修复
  • model_rpc_client.py 的 FetchResponse/Cancel 流程设计合理,含 terminal_seen 状态跟踪和 best-effort Cancel
  • LocalRpcServiceImpl 添加 readyForRegularRpc() 守卫,防止初始化期间的空指针崩溃

…gurable formula engine

- New PrefillTimeFormula (flexlb-sync/strategy/): recursive descent parser
  supporting + - * / ^ (right-assoc), sqrt/log/exp/abs/max/min/pow
- Rewrite PrefillTimePredictor: constructor takes formula string,
  estimateMs/predictBatchMs fill variable bindings then evaluate
- FlexlbConfig: 6 costAlpha* fields replaced by single costFormula
- ConfigService: PREFILL_COEFFICIENTS_ENV → PREFILL_TIME_FORMULA_ENV
- Remove dead code: TaskInfo.estimatePrefillTime, old PrefillTimeFormula
- Update all 5 test files; 23 tests pass
@LLLLKKKK

LLLLKKKK commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/4 · P1/8 · P2/7 · P3/0

Blocking Issues

P0

  • WorkerStatusPB.role 字段类型变更破坏 protobuf 向后兼容 @ rtp_llm/cpp/model_rpc/proto/model_rpc_service.proto:430
    • 建议:应保留旧的 string role = 1 字段(标记 deprecated 或 reserved),使用新的 field number(如 19)添加 RoleTypePB role_type。或者在 C++ 序列化侧同时写入两个字段,反序列化侧优先读新字段、fallback 旧字段,确保滚动部署期间新旧版本互操作正确。
  • RpcCpuTpBroadcaster 使用了错误的地址列表 worker_grpc_addrs @ rtp_llm/cpp/normal_engine/NormalExecutor.cc:115
    • 建议:将 NormalExecutor.cc:115 的 params.runtime_config.worker_grpc_addrs 改为 params.runtime_config.all_worker_grpc_addrs,保持与 RpcCpuTpBroadcaster 内部按 world_rank 索引的语义一致。
  • RpcServerRuntimeMetaTest 单元测试逻辑错误,断言必定失败 @ rtp_llm/cpp/model_rpc/test/RpcServerRuntimeMetaTest.cc:43
    • 建议:方案一:在测试中先调用 meta.enqueuePending(303, 512) 再调用 finishTask,使测试与实现一致。方案二:如果设计意图是 finishTask 在无 pending entry 时也应创建 finished 记录,则修改 finishTask 实现在 running_streams_ 找不到时构造 TaskInfo 并写入 finished_streams_。根据注释 "Request was already dequeued",推荐方案一。
  • master_client_test.py 测试完全过时,未覆盖 gRPC 实现 @ rtp_llm/server/test/master_client_test.py:46
    • 建议:重写 _CaptureMasterClient 以匹配新的 _send_schedule_request(addr, request_pb, timeout_s, request_id) 签名,验证 request_pb 是 FlexlbScheduleRequestPB 实例且其字段正确(request_pb.block_cache_keys, request_pb.seq_len, request_pb.api_key 等),并传入实际的 GenerateInputPB 对象而非 bytes。

P1

  • FlexlbGrpcForwarder 异常处理时移除 channel 但未 shutdown,导致 gRPC 资源泄漏 @ rtp_llm/flexlb/flexlb-api/src/main/java/org/flexlb/httpserver/FlexlbGrpcForwarder.java:63
    • 建议:在 channels.remove(channelKey) 之后,对返回的旧 channel 调用 shutdownNow():ManagedChannel removed = channels.remove(channelKey); if (removed != null) removed.shutdownNow();
  • PrefillEndpoint 在构造函数中启动线程,存在 this 逃逸风险 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/endpoint/PrefillEndpoint.java:44
    • 建议:将 batcher.start() 从构造函数中移出,改为在 EndpointRegistry.ensurePrefillEndpoint() 创建完成后显式调用 start(),或使用工厂方法 + @PostConstruct 模式确保对象完全构造后再启动线程。
  • CostBasedPrefillStrategy 未将 cache 命中率纳入评分,缓存亲和性路由名不副实 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedPrefillStrategy.java:96
    • 建议:将 cache hit 纳入评分公式,例如 score = batcherWaitMs + realWaitTimeMs + predictor.estimateMs(seqLen, cacheHit),使命中率高的 worker 因预估 prefill 时间更短而获得更低的 score。
  • is_backend_service_ready 硬编码返回 True,健康检查失效 @ rtp_llm/server/backend_rpc_server_visitor.py:555
    • 建议:至少保留基本的健康检查(如验证 FlexLB master 地址可达),或者用 disable_domain_fallback 等配置条件控制此行为,而不是无条件返回 True。如果确实要跳过,建议添加 WARNING 日志在启动时提示健康检查已禁用。
  • cache_key_block_size 参数被接受但未传入 gRPC 请求 @ rtp_llm/server/master_client.py:200
    • 建议:确认 FlexLB gRPC 服务端是否仍需要 cache_key_block_size:如果需要,则在 FlexlbScheduleRequestPB proto 中添加该字段并在构建 request_pb 时赋值;如果不需要,则从 get_backend_role_addrs 签名中移除该参数以避免误导。
  • rtp_kernel 可选导入后 None 守卫缺失,运行时将抛出 TypeError @ rtp_llm/ops/fused_rope_kvcache_op.py:53
    • 建议:在调用这些可能为 None 的函数前添加显式检查,例如 if convert_offset_to_block_array is None: raise ImportError("rtp_kernel.fused_rope_kvcache is required"),或在类 init 中提前检查依赖可用性,初始化阶段快速失败。
  • ResponseBufferWriter 队列满时静默丢弃最旧消息 @ rtp_llm/cpp/model_rpc/ResponseBuffer.cc:125
    • 建议:建议在队列满时不静默丢弃,而是返回 false 让调用方感知背压,或记录 WARNING 日志并设置 error_status 通知消费者数据可能不完整。如果 kMaxQueueSize=1000 作为安全阀设计合理,至少应添加 metrics 或日志记录丢弃事件。
  • _FakeMasterClient 签名与真实 MasterClient.get_backend_role_addrs 不匹配 @ rtp_llm/server/test/backend_rpc_server_visitor_test.py:66
    • 建议:将 _FakeMasterClient.get_backend_role_addrs 签名修改为 (self, block_cache_keys, cache_key_block_size, input, request_id, input_pb=None),并在测试断言中验证 cache_key_block_size 的值。

Non-blocking Suggestions

P2

  • EndpointRegistry 只增不删,不会移除下线 worker 端点 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/endpoint/EndpointRegistry.java:47
    • 建议:增加定期扫描逻辑:当 worker 在 EngineWorkerStatus 中已不存在且 endpoint 内无 inflight 条目时,将其从 map 中移除并调用 close()。可在 scheduledEviction() 中实现。
  • admitWaitingUnits 未将 loading 队列纳入 batch 配额计算 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:210
    • 建议:考虑将 countStreams(loading_) 也纳入 admitWaitingUnits 的 running_count 计算,防止 KV 内存被 loading 状态的请求过度占用。如果这是有意设计(提前分配以减少延迟),建议添加注释说明。
  • RpcCpuTpBroadcaster 多线程并发 broadcast 可能导致 seq 不匹配 @ rtp_llm/cpp/distribute/RpcCpuTpBroadcaster.h:75
    • 建议:确认 broadcast() 是否保证单线程串行调用。如果可能多线程并发,建议在 broadcast() 入口添加 assert 或文档说明单线程约束。
  • BatchDecodeScheduler 和 GatherBatchScheduler 中 prepare/isReady 使用 busy-wait 轮询 @ rtp_llm/cpp/engine_base/schedulers/BatchDecodeScheduler.h:86
    • 建议:考虑将 BatchDecodeScheduler 和 GatherBatchScheduler 也改为使用 loading 队列模式,避免 scheduler 线程被 busy-wait 阻塞。或者至少将 sleep 时间增大到 10ms。
  • FixedWindowBatcherAlgorithm 标注 @component 但实际通过 new 创建 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/FixedWindowBatcherAlgorithm.java:33
    • 建议:移除 FixedWindowBatcherAlgorithm 上的 @component 注解,与 SloBudgetBatcherAlgorithm 保持一致。
  • disable_domain_fallback 警告日志在每个请求上重复打印 @ rtp_llm/server/backend_rpc_server_visitor.py:366
    • 建议:将此警告改为在初始化时打印一次,或使用 debug 级别,或使用 log_once 机制避免重复打印。
  • Mega-PR 包含多个独立变更集建议拆分 @ rtp_llm/cpp/engine_base/EngineBase.h:1
    • 建议:建议将 FlexLB Java 组件、C++ 引擎重构、Python gRPC 迁移拆分为独立 PR,便于 review 和 bisect。如果时间不允许拆分,至少确保每个变更集的 commit 是原子的。

Checklist Violations (8 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 兼容性:公开 API/持久数据/配置/环境迁移安全 → issue cache_key_block_size 参数被接受但未传入 gRPC 请求
    _get_backend_role_addrs 接受 cache_key_block_size: int 参数,调用方传入 cache_key_block_size=self.cache_key_block_size(),但构建 FlexlbScheduleRequestPB 时从未使用该参数。Proto 定义中也没有 cache_key_block_size 字段。旧 HTTP 实现通过 JSON payload 传递了此值。FlexLB Java 端 Request.java 仍有 cache_key_block_size 声明,说明服务端可能依赖此字段。
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue RpcCpuTpBroadcaster 多线程并发 broadcast 可能导致 seq 不匹配
    RpcCpuTpBroadcaster 使用全局递增 seq 匹配 root 发送和 non-root 接收。seq_ 使用 fetch_add 保证原子性,但如果 broadcast() 被多线程并发调用,root 和 non-root 的调用顺序不对应时可能导致 seq 不匹配而 timeout。_
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue rtp_kernel 可选导入后 None 守卫缺失,运行时将抛出 TypeError
    当 rtp_kernel.fused_rope_kvcache 导入失败时,convert_offset_to_block_array 等函数被设为 None。但 FusedRopeKVCachePrefillOpBase.prepare() 第53行直接调用 convert_offset_to_block_array(...),无 None 检查。运行时将得到 'NoneType object is not callable' 而非清晰的导入缺失错误。同样问题存在于 cutlass_moe.py 和 cutlass_w4a8_moe.py 中的可选导入。
  • [6.1] Quality — Mega-PR 已拆分为独立变更 → issue Mega-PR 包含多个独立变更集建议拆分
    此 PR 包含 195 个文件、至少三个大型独立变更集:(1) FlexLB Java 负载均衡组件(约 100 个新 Java 文件),(2) C++ 引擎层 GenerateStream/Scheduler/RpcCpuTpBroadcaster 重构(约 50 个 C++/头文件),(3) Python FlexLB gRPC 客户端迁移 + 配置扩展(约 30 个 Python 文件)。这些变更集虽然有关联但可以独立 review 和测试。
  • [6.1] Quality — 无 per-forward 调试日志 / 噪声热路径输出 → issue disable_domain_fallback 警告日志在每个请求上重复打印
    route_ips() 方法在每次调用时,只要 disable_domain_fallback 为 True,就会打印 WARNING 'skipping domain fallback routing'。在高 QPS 场景下会产生大量重复日志,影响日志可读性和性能。
  • [6.1] Software Engineering — KISS/YAGNI:无投机性抽象 → issue FixedWindowBatcherAlgorithm 标注 @Component 但实际通过 new 创建
    FixedWindowBatcherAlgorithm 类标注了 @component,Spring 容器会创建单例 bean。但 WorkerBatcher.createAlgorithm() 通过 new 直接创建实例。SloBudgetBatcherAlgorithm 未标注 @component(正确做法)。多余的 @component 会产生一个从未使用的 Spring bean。
  • [6.1] Software Engineering — SRP:模块/类职责单一 → issue CostBasedPrefillStrategy 未将 cache 命中率纳入评分,缓存亲和性路由名不副实
    doSelect() 选择循环中 computeScore(ep) 仅计算 batcherWaitMs() + realWaitTimeMs(),完全不使用已计算的 cacheHit 值。cacheHit 仅用于 SLO hard filter 中的 singlePrefillMs 预估和 DebugInfo 填充,不影响 worker 选择排序。两个 waitMs 相同但 cache hit 差距巨大的 worker(0% vs 90%)会被视为等价,无法实现缓存亲和性。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue _FakeMasterClient 签名与真实 MasterClient.get_backend_role_addrs 不匹配
    __FakeMasterClient.get_backend_role_addrs 签名为 (block_cache_keys, input, request_id, input_pb_bytes=None),缺少 cache_key_block_size 参数且参数名 input_pb_bytes 与真实方法的 input_pb 不同。真实调用方传递 cache_key_block_size=self.cache_key_block_size() 和 input_pb=input_pb。由于 Python duck typing,这个 fake 能跑通,但 cache_key_block_size 实际被丢弃。

Strengths

  • GenerateStream 生命周期方法 (prepare/isReady/activate/advance/alive/finish) 设计清晰内聚,将原先分散在 GenerateStateMachine 多个 handle* 方法中的逻辑集中到 GenerateStream 本身,每个方法职责单一、锁策略一致
  • ScheduleUnit 封装分组调度逻辑,FIFOScheduler 的三阶段调度 (waiting → loading → running) 结构清晰,每个阶段的错误处理和死亡检查完整
  • FlexLB BatchScheduler / BatchDispatcher / WorkerBatcher 职责分离清晰,通过 BatchDecisionHandler / DispatchCallback 接口解耦,便于测试和替换算法
  • PrefillTimeFormula 实现完整的递归下降解析器,支持可配置公式引擎,无需改代码即可调整 prefill 时间预估模型
  • ResponseBuffer + WriterInterface 将流式响应从 gRPC ServerWriter 中抽象出来,使 EnqueueBatch/FetchResponse 异步模式可复用已有 pollStreamOutput 流程
  • FlexLB cancel 路径实现了完整的双层同步:synchronized(entry) + AtomicBoolean cancelled 保证 cancel 和 ack 竞态安全
  • 新增的 access log(prefill_batch_begin/end、decode_step_begin、first_token、decode_finished)为生产环境调试提供关键可观测性数据
  • Proto 字段重命名(batch_group_* → group_*)正确保留了 field number 不变,确保线格式兼容;已删除字段使用了 reserved 关键字

wzy-99 added 3 commits July 2, 2026 15:44
…ache AND logic

stopStream() no longer races with Engine Loop's advance() for KV cache
persistence. Instead of actively calling finish() or reportError(),
stopStream() now passively waits for Engine Loop to naturally detect
GenerateDone → finish_internal() → FINISHED → insertIntoCache().

Changes:
- stopStream(): when GenerateDone && !hasError, silently wait for
  Engine Loop to set FINISHED; only reportError for genuine errors
- add prefill_stop_stream_wait_timeout_ms (default 2000ms) with
  timeout-based fallback to prevent indefinite wait
- StreamCacheResource::reuseCache(): restore AND logic — both
  global REUSE_CACHE=1 AND per-request reuse_cache must be true
- wire new config through Python CLI args, ConfigModules, and
  PrefillGenerateContext constructor in both GenerateStreamCall
  and EnqueueGroup paths

This eliminates the race where stopStream()'s reportError(CANCELLED)
could set hasError=true before finish_internal() could set FINISHED,
blocking insertIntoCache and causing KV cache items=0 in PD prefill.
LoadBalancer.rollBack(String ipPort, long requestId)
  → LoadBalancer.rollBack(WorkerEndpoint ep, long requestId)

- DefaultRouter 注入 EndpointRegistry,在 rollBackRoutingFailure 中统一
  查 endpoint 后再传给策略,策略不再各自持有 registry 引用
- RandomStrategy 彻底删除从未使用的 endpointRegistry 字段
- CostBasedPrefillStrategy/CostBasedDecodeStrategy 用 instanceof
  替代 registry 查表,减少依赖
- 同步更新 4 个测试文件的构造器和 verify 调用

Tests: 48 passed, 0 failures
@LLLLKKKK

LLLLKKKK commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/1 · P1/8 · P2/5 · P3/0

Blocking Issues

P0

  • master_client_test.py 测试签名与重构后代码完全不匹配,测试必然失败 @ rtp_llm/server/test/master_client_test.py:46
    • 建议:将 _CaptureMasterClient._send_schedule_request 签名改为 (self, addr, request_pb, timeout_s, request_id) 以匹配重构后接口。test_schedule_payload_contains_batch_fields_and_pb 中补充 cache_key_block_size 参数,将 input_pb_bytes 改为 input_pb 并传入 protobuf mock 对象。验证 request_pb 而非 payload dict 的字段。

P1

  • 超大 PR(mega-PR):197 文件、14K+ 行变更、6 个独立功能领域混合 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:1
    • 建议:建议拆分为至少 4 个独立 PR:(1) FlexLB Java batch 调度,(2) C++ scheduler/stream/PrefillRpcServer 重构,(3) 跨节点 CPU TP broadcast,(4) Python 服务端重构 + 配置变更。各 PR 独立 CI、独立回滚。同时清理 fix-then-revert 对(commit 5e4649 + 629b624)。
  • WorkerStatusPB.role 从 string 改为 enum 是 protobuf wire format 不兼容变更 @ rtp_llm/cpp/model_rpc/proto/model_rpc_service.proto:430
    • 建议:如果存在滚动部署场景,应使用新 field number 承载 RoleTypePB 类型的 role,并 reserve 旧 field 1 + 名称。如果确认所有端同步发布,应在 PR description 中明确标注 breaking change 并确保上下游无遗漏。
  • RuntimeConfig 和 PDSepConfig pickle 格式不兼容旧版本 @ rtp_llm/cpp/pybind/ConfigInit.cc:1305
    • 建议:在 unpickle 中增加对旧 size 的兼容处理:RuntimeConfig size==13 时 all_worker_grpc_addrs 使用空 vector、specify_gpu_arch 从 t[12] 读取;PDSepConfig size>=20 但<26 时新增字段使用 C++ 默认值(60000, 10000, 10000, 0, 0, 0)。
  • cp_force_single_prefill 和 max_inited_kv_cache_streams 功能被静默删除 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:17
    • 建议:确认这两个功能是否应保留。如果是有意移除,应同步删除 FIFOSchedulerConfig 中对应字段和相关测试(如 testCpForceSinglePrefillConfig);如果无意遗漏,应在 canAdmitUnitadmitWaitingUnits 中恢复限制逻辑。
  • is_backend_service_ready 被硬编码为 return True,健康检查失效 @ rtp_llm/server/backend_rpc_server_visitor.py:555
    • 建议:即使希望强制 FlexLB 路由,也应在健康检查中增加对 FlexLB master 连通性的基本检查(如 gRPC channel 可达性),而非无条件 True。至少应添加可配置开关控制此行为,并在代码注释中说明安全性论证。
  • backend_rpc_server_visitor_test.py 中 fake client 签名与实际不匹配 @ rtp_llm/server/test/backend_rpc_server_visitor_test.py:66
    • 建议:将 _FakeMasterClient.get_backend_role_addrs 签名更新为 (self, block_cache_keys, cache_key_block_size, input, request_id, input_pb=None) 以匹配实际接口。
  • FlexLB ExpirationCleaner 移除过期 worker 时未清理 EndpointRegistry 中对应 endpoint @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/schedule/ExpirationCleaner.java:49
    • 建议:ExpirationCleaner 应注入 EndpointRegistry,当移除 WorkerStatus 时同步调用 EndpointRegistry 移除对应 endpoint(并触发 close/shutdown batcher 线程)。或在 EndpointRegistry 中增加 removeByIpPort(String) 方法由 ExpirationCleaner 统一调用。
  • CostBasedDecodeStrategy.buildServerStatus 中 reserve 后异常不 release,inflight 泄漏 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedDecodeStrategy.java:253
    • 建议:在 catch 块中增加 if (optimalEndpoint != null) optimalEndpoint.release(requestId) 调用。或将 reserve 移到所有 setter 之后、确认无异常后再预留资源。

Non-blocking Suggestions

P2

  • rtp_kernel 可选导入设为 None 后调用产生无意义 TypeError @ rtp_llm/ops/fused_rope_kvcache_op.py:19
    • 建议:在每个使用点添加 None 检查并抛出明确的 RuntimeError(如 "rtp_kernel.fused_rope_kvcache is required but not available"),或在类 __init__ 中提前检查并报错。
  • ScheduleUnit group 流部分失败时不完整 group 继续执行 @ rtp_llm/cpp/engine_base/schedulers/ScheduleUnit.h:13
    • 建议:对 isGroup() 为 true 的 ScheduleUnit,当任何流 prepare/activate 失败时,应取消(reportError + finish)该 unit 中所有其他流,而不是仅移除失败流。
  • SloBudgetBatcherAlgorithm(396 行核心调度逻辑)缺少单元测试 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/SloBudgetBatcherAlgorithm.java:1
    • 建议:为 SloBudgetBatcherAlgorithm 添加专门的单元测试类,至少覆盖:deadline expired 路径、inflight_full_guard drop 路径、targetBatchSize 在不同 arrival rate 下的计算、shouldWaitForMore 边界条件、EMA 冷启动和稳态行为。
  • pyi 类型存根缺少 PDSepConfig 新属性 @ rtp_llm/ops/libth_transformer_config.pyi:1320
    • 建议:在 libth_transformer_config.pyi 的 PDSepConfig 类中补充所有 6 个新属性声明。
  • FlexlbGrpcForwarder 错误路径移除 channel 时未调用 shutdown @ rtp_llm/flexlb/flexlb-api/src/main/java/org/flexlb/httpserver/FlexlbGrpcForwarder.java:63
    • 建议:在 channels.remove 之后调用 channel.shutdown()channel.shutdownNow()

Checklist Violations (8 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 兼容性:公开 API/持久数据/配置/环境迁移安全 → issue pyi 类型存根缺少 PDSepConfig 新属性
    ConfigInit.cc 通过 def_readwrite 绑定了 prefill_enqueue_pool_sizeprefill_worker_lambda_pool_sizeprefill_slot_pool_size 三个属性,但 libth_transformer_config.pyi 中 PDSepConfig 类声明缺少这三个字段。pyright strict 模式下访问这些属性会报类型错误。同样 batch_dispatch_timeout_msbatch_prepare_timeout_msbatch_load_timeout_ms 也可能缺失。
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue FlexlbGrpcForwarder 错误路径移除 channel 时未调用 shutdown
    catch (StatusRuntimeException) 和 catch (Exception) 两处均执行 channels.remove(channelKey) 但未对移除的 ManagedChannel 调用 shutdown()。每次转发失败后泄漏一个 gRPC channel(含 Netty EventLoop 线程和 TCP 连接),长时间运行会导致 fd 耗尽。对比 AbstractGrpcClient.updateGrpcChannelPool 中 remove 后均有 invoker.shutdown()
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue CostBasedDecodeStrategy.buildServerStatus 中 reserve 后异常不 release,inflight 泄漏
    buildServerStatus 第 253 行调用 optimalEndpoint.reserve(requestId, seqLen) 在 DecodeEndpoint 的 inflightRequests 中预留 KV 空间。但 catch 块(264-268行)只设置错误信息并返回 result.setSuccess(false),没有调用 optimalEndpoint.release(requestId)。如果 reserve 之后的代码抛异常,已预留的 inflight 不会释放。调用方 DefaultRouter 的 rollback 路径依赖 serverStatus 中的 role 字段查找 LoadBalancer,但异常时 role 可能未设置。
  • [6.1] Quality — Mega-PR 已拆分为独立变更 → issue 超大 PR(mega-PR):197 文件、14K+ 行变更、6 个独立功能领域混合
    PR 横跨 6 个独立功能领域:FlexLB Java batch 调度(~105 文件/8700+ 行)、C++ scheduler/stream 状态机重构(FIFOScheduler 重写)、PrefillRpcServer 大改(+1621 行)、跨节点 CPU TP broadcast 全新特性(18 文件/+676 行、commit c92beed)、Python master_client 重构(+297 行)、kernel import 容错(4 文件)。这些功能耦合度低,完全可独立提交和回滚。其中 commit 63e65ed 一次性修改 174 个文件(+13023 行),包含所有领域的混合改动。
  • [6.1] Quality — 逻辑变更未混入无关格式化 → issue 超大 PR(mega-PR):197 文件、14K+ 行变更、6 个独立功能领域混合
    PR 横跨 6 个独立功能领域:FlexLB Java batch 调度(~105 文件/8700+ 行)、C++ scheduler/stream 状态机重构(FIFOScheduler 重写)、PrefillRpcServer 大改(+1621 行)、跨节点 CPU TP broadcast 全新特性(18 文件/+676 行、commit c92beed)、Python master_client 重构(+297 行)、kernel import 容错(4 文件)。这些功能耦合度低,完全可独立提交和回滚。其中 commit 63e65ed 一次性修改 174 个文件(+13023 行),包含所有领域的混合改动。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue SloBudgetBatcherAlgorithm(396 行核心调度逻辑)缺少单元测试
    SloBudgetBatcherAlgorithm(396 行)是批量调度核心最复杂的组件,包含 EMA 到达率估计、budget-based 贪心填充、deadline-gated dispatch、inflight 背压等关键逻辑。在测试目录中不存在 SloBudgetBatcherAlgorithmTest.javaFixedWindowBatcherAlgorithm 有 108 行测试作为对比。deadline 恰好过期、inflight 满载、arrival rate 冷启动、targetBatchSize 计算等边界场景均无覆盖。
  • [6.1] Tests — 被删除测试有等价替代覆盖 → issue cp_force_single_prefill 和 max_inited_kv_cache_streams 功能被静默删除
    FIFOScheduler 重构中删除了 cp_force_single_prefill_ 成员变量和 max_inited_kv_cache_streams_ 限制逻辑及 countInitedKVCacheStreams() 方法,但对应的 FIFOSchedulerConfig 配置字段仍然存在。cp_force_single_prefill 原本限制 Context-Parallel prefill 模式每轮只调度一个流,max_inited_kv_cache_streams 限制同时已分配 KV cache 的流数防止 OOM。如果生产环境依赖这些配置,删除后可能导致 CP prefill 正确性问题或 KV cache 内存超限。

Python Static-First Checklist

  • [P.B] 错误处理 — 禁止 bare except 或静默吞异常 → issue rtp_kernel 可选导入设为 None 后调用产生无意义 TypeError
    rtp_kernel.fused_rope_kvcache import 失败时,convert_offset_to_block_arraydecode_fused_rope_kvcacheprefill_fused_rope_kvcache 被设为 None。后续代码(:53、:98、:192、:230)直接调用这些函数无 None 检查。调用 None 抛出 TypeError: 'NoneType' object is not callable,错误信息不明确。同样问题存在于 cutlass_moe.py 和 cutlass_w4a8_moe.py。

Strengths

  • ScheduleUnit 抽象和 prepare/isReady/activate/advance/finish 生命周期方法的引入,将状态机从集中式 moveToNext() 拆分为调度器直接驱动,代码结构更清晰可测试
  • FlexLB 批量调度架构设计良好:FlexlbBatchScheduler 协调器 + WorkerBatcher 队列 + BatcherAlgorithm 策略接口 + DefaultBatchDispatcher gRPC I/O,职责分离明确
  • FIFOScheduler 新增 7 个 group 隔离测试和 2 个 token cap 测试,覆盖了 group 隔离、singles 混合、双 group 隔离等场景
  • master_client.py 从 aiohttp HTTP 到 gRPC 的重构完整,channel 管理和错误处理合理
  • Proto 字段重命名正确保留了 field number(wire 格式兼容),deleted 字段使用 reserved 声明防止冲突
  • ResponseBufferRegistry 线程安全设计合理:registry 级锁和 entry 级锁分离,cancelAll 释放 registry 锁后再执行回调避免死锁
  • PrefillEndpoint.calibrate 实现了完整的 5 阶段状态校准逻辑,与引擎侧状态保持一致

wzy-99 added 4 commits July 2, 2026 23:56
…y score

Pass cacheHit and seqLen into computeScore to estimate per-request
prefill time via PrefillTimePredictor. This makes cache match results
directly participate in worker ranking, instead of being used only
for SLO filtering and metrics reporting.

score = predictor.estimateMs(seqLen, cacheHit) + batcherWaitMs() + realWaitTimeMs()
…lock key in updateCacheKeys

FlexLB: WorkerStatus.updateFromResponse() unconditionally overwrote
cacheStatus with resp.getCacheStatus(), which is null in GetWorkerStatus
responses (cache status is only populated by GetCacheStatus gRPC).
This nullified the cacheStatus set by GrpcCacheStatusCheckRunner on every
status sync round, causing calculateCacheHit() to always return 0 even
when GlobalCacheIndex had a successful prefix match — hit_cache was 0.

Fix: only update cacheStatus when resp.getCacheStatus() is non-null.

C++: updateCacheKeys() dropped the partial block key after incremental
update and hard-coded lastBlockAligned=true. For sequences shorter than
one block this produced zero cache keys, causing insertIntoCache to skip
device cache insertion entirely.

Fix: re-compute the partial block key when remaining tokens exist, and
set lastBlockAligned consistent with initCacheKeys behavior.
FlexLB theory hit ratio (whale-lb.app.cache.theory.hit.ratio) was always 0
because cache_key_block_size was not included in FlexlbScheduleRequestPB,
causing RecentCacheKeyTraceReporter.theoryHitTokens() to short-circuit at
cacheKeyBlockSize <= 0.

- Add cache_key_block_size field (field 13) to FlexlbScheduleRequestPB proto
- Pass cache_key_block_size from Python master_client.py when building the request
- Set cacheKeyBlockSize in FlexlbServiceImpl.buildContext() from proto field
- Add test verifying cacheKeyBlockSize propagation through buildContext
…Server thread pools

- Add PrefillPoolMetrics MetricsGroup (active/queued/completed/rejected/fallback/thread_max/queue_max)
- Wire all 5 call sites with ScopeExit RAII guards for accurate active/completed tracking
- Add reportOnePoolToKmonitor() called every 10s from GC thread
- Demote periodic PoolMetrics log from INFO to DEBUG to avoid production noise
- Add thread_max/queue_max to PoolMetrics for utilization monitoring
@LLLLKKKK

LLLLKKKK commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/1 · P1/8 · P2/5 · P3/0

Blocking Issues

P0

  • master_client_test.py 测试签名与重构后 MasterClient 完全不匹配,测试必然 TypeError @ rtp_llm/server/test/master_client_test.py:46
    • 建议:将 _CaptureMasterClient._send_schedule_request 签名更新为 (self, addr, request_pb, timeout_s, request_id) 并存储/断言 protobuf 字段而非 JSON dict;测试用例补齐 cache_key_block_size 并将 input_pb_bytes 改为 input_pb,注入 protobuf mock。或者删除本测试文件并新增覆盖新接口的用例。

P1

  • backend_rpc_server_visitor_test.py 中 _FakeMasterClient 签名与实际不匹配 @ rtp_llm/server/test/backend_rpc_server_visitor_test.py:66
    • 建议:将 fake 签名更新为 (self, block_cache_keys, cache_key_block_size, input, request_id, input_pb=None);断言部分同步调整从 input_pb_bytes 改为 input_pb
  • 超大 mega-PR:201 文件 / ~19K 行 / 6 个独立功能领域混合 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:1
    • 建议:建议拆分为至少 4 个独立 PR:(1) FlexLB Java batch 调度框架,(2) C++ scheduler/stream/PrefillRpcServer 重构,(3) 跨节点 CPU TP broadcast,(4) Python master_client gRPC 迁移 + 其余配置/pickle 变更。每个 PR 独立跑 CI 与 smoke,独立可回滚。
  • WorkerStatusPB.role 从 string 改为 enum 是 protobuf wire 不兼容变更 @ rtp_llm/cpp/model_rpc/proto/model_rpc_service.proto:431
    • 建议:如果存在滚动升级/跨节点异版本互通场景,应引入新 field number 承载 RoleTypePB 类型的 role,并对旧 field 1 使用 reserved 1; + reserved "role";;如果确认所有端同步发布,应在 PR description 中明确声明 breaking change,并在 release note 中列出所有需同步升级的下游。
  • RuntimeConfig / PDSepConfig pickle 格式不向后兼容旧版本 @ rtp_llm/cpp/pybind/ConfigInit.cc:1305
    • 建议:采用与 PrefillCPConfig 同样的“字段可选、按 size 分层解析”兼容策略:RuntimeConfig 支持 size >= 13all_worker_grpc_addrs 使用空 vector;PDSepConfig 支持 size >= 20 时新增 6 个字段使用 C++ 默认值(60000/10000/10000/0/0/0)。
  • FIFOScheduler 静默删除 cp_force_single_prefillmax_inited_kv_cache_streams 语义 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:1
    • 建议:明确删除意图:若有意移除,同步删除 config 字段/pybind binding/pickle 中对应位置,并删除或改写 testCpForceSinglePrefillConfig 等测试;若为遗漏,恢复 admitWaitingUnits/canAdmitUnit 中的两项限制。PR description 应记录此语义变更以便 rollout 前审查。
  • is_backend_service_ready 被硬编码 return True,健康检查失效 @ rtp_llm/server/backend_rpc_server_visitor.py:555
    • 建议:至少保留一项对 FlexLB master gRPC 连通性或 host_service.service_available 的基本检查,返回真实健康状态;或引入显式配置开关(默认 false)控制“强制 True”行为,并在开启路径添加 warn 日志,避免长期无 tag 隐蔽风险。
  • FlexLB ExpirationCleaner 移除 WorkerStatus 时未清理 EndpointRegistry 中对应 endpoint @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/sync/schedule/ExpirationCleaner.java:41
    • 建议:在 EndpointRegistry 增加 removeByIpPort(String ipPort) 方法,close 并 remove 对应 endpoint;ExpirationCleaner 注入 EndpointRegistry,it.remove() 之后立即调用。或改用 EndpointRegistry 作为唯一入口,putEndpoint/ensureEndpoint/removeEndpoint 触达 WorkerStatus。
  • CostBasedDecodeStrategy.buildServerStatus 中 reserve 之后异常不 release,inflight 泄漏 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/strategy/CostBasedDecodeStrategy.java:253
    • 建议:在 catch 中显式判断 if (optimalEndpoint != null) optimalEndpoint.release(requestId);或将 reserve 移到所有 setter 完成、确认无异常后再执行("最后 commit")。

Non-blocking Suggestions

P2

  • rtp_kernel 可选 import 失败被静默设为 None,调用点抛无信息 TypeError @ rtp_llm/ops/fused_rope_kvcache_op.py:19
    • 建议:在 init 中提前校验:若对应函数为 None,抛 RuntimeError("rtp_kernel.fused_rope_kvcache is required for ... but not installed: <import_err>");或去掉 try/except,改为 lazy import 并在真正调用点 raise 明确错误。
  • ScheduleUnit group 语义在部分流失败时被破坏 @ rtp_llm/cpp/engine_base/schedulers/ScheduleUnit.h:13
    • 建议:针对 isGroup() 为 true 的 unit,当任意流 prepare/activate/advance 失败时,主动对同 unit 内其他流调用 reportError + finish(原子失败),避免带残缺 group 进入调度下游。
  • SloBudgetBatcherAlgorithm(396 行核心调度)缺少专门单元测试 @ rtp_llm/flexlb/flexlb-sync/src/main/java/org/flexlb/balance/scheduler/SloBudgetBatcherAlgorithm.java:1
    • 建议:补齐 SloBudgetBatcherAlgorithmTest,至少覆盖:deadline 到达/未到达、inflight_full_guard drop 路径、targetBatchSize 在低/中/高 arrival rate 下的取值、EMA 冷启动与稳态、shouldWaitForMore 边界。
  • libth_transformer_config.pyi 缺少 PDSepConfig 3 个新 pool 字段类型声明 @ rtp_llm/ops/libth_transformer_config.pyi:1320
    • 建议:在 .pyi 的 PDSepConfig 类中按字母序补齐 prefill_enqueue_pool_size: intprefill_slot_pool_size: intprefill_worker_lambda_pool_size: int
  • FlexlbGrpcForwarder 错误路径 remove channel 时未 shutdown,Netty EventLoop/连接泄漏 @ rtp_llm/flexlb/flexlb-api/src/main/java/org/flexlb/httpserver/FlexlbGrpcForwarder.java:63
    • 建议:将 channels.remove(channelKey) 改为 ManagedChannel old = channels.remove(channelKey); if (old != null) old.shutdown();,并考虑抽取 helper 保证一致语义。

Checklist Violations (14 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 兼容性:公开 API/持久数据/配置/环境迁移安全 → issue libth_transformer_config.pyi 缺少 PDSepConfig 3 个新 pool 字段类型声明
    ConfigInit.cc 为 PDSepConfig 新增 .def_readwrite 绑定 prefill_enqueue_pool_sizeprefill_worker_lambda_pool_sizeprefill_slot_pool_size(ConfigInit.cc:1722-1724);libth_transformer_config.pyi 中 PDSepConfig 只声明了 batch_dispatch/prepare/load_timeout_ms 三个新字段(:1321-1323),缺少上述三个 pool 字段。pyright strict 下 Python 代码访问 pd_sep_config.prefill_enqueue_pool_size 等属性会报类型错误。
  • [6.1] Architecture — 可观测性:日志/指标/超时可操作、非噪声 → issue FlexlbGrpcForwarder 错误路径 remove channel 时未 shutdown,Netty EventLoop/连接泄漏
    catch StatusRuntimeException(:60-64)和 catch Exception(:65-70)分支均执行 channels.remove(channelKey) 但未对返回的 ManagedChannel 调用 shutdown()/shutdownNow()。每次转发失败即泄漏一个 ManagedChannel(包含 gRPC 内部 executor、Netty 相关引用、TCP fd),长时间/高错误率下可能 fd 或线程/内存耗尽。对比 AbstractGrpcClient.updateGrpcChannelPool 中 remove 后均有 shutdown。
  • [6.1] Architecture — 回滚路径:风险行为存在运维回滚手段 → issue ``is_backend_service_ready 被硬编码 return True,健康检查失效
    `is_backend_service_ready` 方法(:555-568)所有实际检查逻辑被注释,无条件 `return True`,注释 “Always return True to force routing through FlexLB”。该方法被 `frontend/frontend_server.py:524` 的 `/health` 端点直接调用。后果:即使后端完全不可达、gRPC channel 全部失败或 FlexLB master 挂掉,前端仍报告 healthy,L7 LB / K8s liveness/readiness 无法识别故障节点,流量继续注入,故障放大。
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue FlexlbGrpcForwarder 错误路径 remove channel 时未 shutdown,Netty EventLoop/连接泄漏
    catch StatusRuntimeException(:60-64)和 catch Exception(:65-70)分支均执行 channels.remove(channelKey) 但未对返回的 ManagedChannel 调用 shutdown()/shutdownNow()。每次转发失败即泄漏一个 ManagedChannel(包含 gRPC 内部 executor、Netty 相关引用、TCP fd),长时间/高错误率下可能 fd 或线程/内存耗尽。对比 AbstractGrpcClient.updateGrpcChannelPool 中 remove 后均有 shutdown。
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue rtp_kernel 可选 import 失败被静默设为 None,调用点抛无信息 TypeError
    try: from rtp_kernel.fused_rope_kvcache import ...; except: 三函数设为 None(:14-22),然后类 PrefillFusedRopeKVCacheOp / DecodeFusedRopeKVCacheOpcall 内直接调用 prefill_fused_rope_kvcache(...) / decode_fused_rope_kvcache(...)(:53, :98, :192, :230)。若 kernel 缺失,运行时抛 TypeError: 'NoneType' object is not callable,无法定位真实原因。cutlass_moe.py / cutlass_w4a8_moe.py 同模式。
  • [6.1] Quality — Commit 原子、message 与行为匹配 → issue 超大 mega-PR:201 文件 / ~19K 行 / 6 个独立功能领域混合
    PR 混合 6 个独立特性:FlexLB Java batch 调度框架(~105 文件、+8K 行)、C++ scheduler/stream/state machine 重构(含 FIFOScheduler 重写与 ScheduleUnit 新抽象)、PrefillRpcServer 大改(+1.6K 行)、跨节点 CPU TP broadcast 全新特性(RpcCpuTpBroadcaster + 环境开关 RTP_LLM_CROSS_NODE_CPU_TP_BROADCAST)、Python master_client 从 HTTP 迁移到 gRPC、kernel 可选 import 容错(cutlass_moe/w4a8/fused_rope)。这些领域耦合度低、CI 可独立通过、可独立回滚;混在同一 PR 中导致回滚粒度过粗、code review 覆盖不充分、bisect 定位困难。
  • [6.1] Quality — Mega-PR 已拆分为独立变更 → issue 超大 mega-PR:201 文件 / ~19K 行 / 6 个独立功能领域混合
    PR 混合 6 个独立特性:FlexLB Java batch 调度框架(~105 文件、+8K 行)、C++ scheduler/stream/state machine 重构(含 FIFOScheduler 重写与 ScheduleUnit 新抽象)、PrefillRpcServer 大改(+1.6K 行)、跨节点 CPU TP broadcast 全新特性(RpcCpuTpBroadcaster + 环境开关 RTP_LLM_CROSS_NODE_CPU_TP_BROADCAST)、Python master_client 从 HTTP 迁移到 gRPC、kernel 可选 import 容错(cutlass_moe/w4a8/fused_rope)。这些领域耦合度低、CI 可独立通过、可独立回滚;混在同一 PR 中导致回滚粒度过粗、code review 覆盖不充分、bisect 定位困难。
  • [6.1] Quality — 逻辑变更未混入无关格式化 → issue 超大 mega-PR:201 文件 / ~19K 行 / 6 个独立功能领域混合
    PR 混合 6 个独立特性:FlexLB Java batch 调度框架(~105 文件、+8K 行)、C++ scheduler/stream/state machine 重构(含 FIFOScheduler 重写与 ScheduleUnit 新抽象)、PrefillRpcServer 大改(+1.6K 行)、跨节点 CPU TP broadcast 全新特性(RpcCpuTpBroadcaster + 环境开关 RTP_LLM_CROSS_NODE_CPU_TP_BROADCAST)、Python master_client 从 HTTP 迁移到 gRPC、kernel 可选 import 容错(cutlass_moe/w4a8/fused_rope)。这些领域耦合度低、CI 可独立通过、可独立回滚;混在同一 PR 中导致回滚粒度过粗、code review 覆盖不充分、bisect 定位困难。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue SloBudgetBatcherAlgorithm(396 行核心调度)缺少专门单元测试
    SloBudgetBatcherAlgorithm(396 行)是 batch 调度核心组件,包含 EMA arrival rate 估计、budget-based greedy fill、deadline-gated dispatch、inflight 背压等多阶段逻辑。测试目录(rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/)不存在 SloBudgetBatcherAlgorithmTest.java;同级 FixedWindowBatcherAlgorithm 有测试作对比。deadline 恰好过期、inflight 满载、arrival rate 冷启动、targetBatchSize 计算等分支均无覆盖,回归风险高。
  • [6.1] Tests — 被删除测试有等价替代覆盖 → issue FIFOScheduler 静默删除 cp_force_single_prefillmax_inited_kv_cache_streams 语义
    FIFOSchedulerConfig 保留 cp_force_single_prefill(默认 true)与 max_inited_kv_cache_streams(默认 0)两个字段(ConfigModules.h:376),pybind 也仍然导出(ConfigInit.cc:1202),pickle 继续序列化;但重构后 FIFOScheduler.cc / .h 不再引用这两个字段,countInitedKVCacheStreams() 已删除、CP prefill 单流限制逻辑不再存在。测试 FIFOSchedulerTest.cc:1122 仍设置 cp_force_single_prefill = true,但被调度器忽略——测试可能仍通过,但功能已失效。生产上若线上依赖这两个配置来避免 CP prefill 竞态或 KV 内存超限,则升级后可能观察到 CP 正确性问题或 OOM。
  • [6.1] Tests — 边界 case 覆盖(空、单元素、最大值) → issue SloBudgetBatcherAlgorithm(396 行核心调度)缺少专门单元测试
    SloBudgetBatcherAlgorithm(396 行)是 batch 调度核心组件,包含 EMA arrival rate 估计、budget-based greedy fill、deadline-gated dispatch、inflight 背压等多阶段逻辑。测试目录(rtp_llm/flexlb/flexlb-sync/src/test/java/org/flexlb/balance/scheduler/)不存在 SloBudgetBatcherAlgorithmTest.java;同级 FixedWindowBatcherAlgorithm 有测试作对比。deadline 恰好过期、inflight 满载、arrival rate 冷启动、targetBatchSize 计算等分支均无覆盖,回归风险高。

Python Static-First Checklist

  • [P.B] 错误处理 — 禁止 bare except 或静默吞异常 → issue rtp_kernel 可选 import 失败被静默设为 None,调用点抛无信息 TypeError
    try: from rtp_kernel.fused_rope_kvcache import ...; except: 三函数设为 None(:14-22),然后类 PrefillFusedRopeKVCacheOp / DecodeFusedRopeKVCacheOpcall 内直接调用 prefill_fused_rope_kvcache(...) / decode_fused_rope_kvcache(...)(:53, :98, :192, :230)。若 kernel 缺失,运行时抛 TypeError: 'NoneType' object is not callable,无法定位真实原因。cutlass_moe.py / cutlass_w4a8_moe.py 同模式。
  • [P.G] 测试规范 — mock.patch target 是使用处而非定义处 → issue master_client_test.py 测试签名与重构后 MasterClient 完全不匹配,测试必然 TypeError
    _CaptureMasterClient._send_schedule_request 覆写签名为 (self, addr, payload, generate_timeout_ms, request_id, request_headers=None),对应旧 HTTP/JSON 接口;而 MasterClient._send_schedule_request 重构后为 (self, addr, request_pb, timeout_s, request_id)(master_client.py:159),参数名/个数/语义完全不同。同一测试第 78-83 行调用 get_backend_role_addrs(block_cache_keys=..., input=..., request_id=99, input_pb_bytes=b'serialized-input'),但真实方法签名 (self, block_cache_keys, cache_key_block_size, input, request_id, input_pb=None) 需要 `c
  • [P.H] 类型标注 — Optional[X] 表示可为 None 而非参数可选 → issue libth_transformer_config.pyi 缺少 PDSepConfig 3 个新 pool 字段类型声明
    ConfigInit.cc 为 PDSepConfig 新增 .def_readwrite 绑定 prefill_enqueue_pool_sizeprefill_worker_lambda_pool_sizeprefill_slot_pool_size(ConfigInit.cc:1722-1724);libth_transformer_config.pyi 中 PDSepConfig 只声明了 batch_dispatch/prepare/load_timeout_ms 三个新字段(:1321-1323),缺少上述三个 pool 字段。pyright strict 下 Python 代码访问 pd_sep_config.prefill_enqueue_pool_size 等属性会报类型错误。

Strengths

  • ScheduleUnit 抽象 + prepare/isReady/activate/advance/finish 生命周期方法引入,将状态机由集中式 moveToNext 拆分为调度器直接驱动,可测试性提升
  • FlexLB 批量调度分层清晰:FlexlbBatchScheduler 协调器 + WorkerBatcher 队列 + BatcherAlgorithm 策略接口 + DefaultBatchDispatcher gRPC I/O,各层职责单一
  • FIFOScheduler 新增大量 group 隔离/token cap 测试,覆盖多 group、singles 混合、token cap 边界等场景
  • Proto 字段重命名/删除均使用 reserved 声明保留 field number,避免未来复用冲突
  • ResponseBufferRegistry 采用 registry 级锁 + entry 级锁分离,cancelAll 释放 registry 锁后再执行回调,避免死锁
  • PrefillEndpoint.calibrate 实现完整的 5 阶段状态校准,与引擎侧状态保持一致

wzy-99 and others added 2 commits July 3, 2026 13:53
Problem: FlexlbBatchScheduler.cancel() only calls cancelPrefill (gRPC
cancel) and rollbackOnce (releases DecodeEndpoint resources), but never
removes the request from PrefillEndpoint.inflightBatches. When
FLEXLB_BATCH_FIXED_MAX_INFLIGHT_BATCHES is enabled, this causes the
inflight count to grow monotonically on cancels, triggering false
backpressure and starving the engine.

Changes in FlexlbBatchScheduler.java:

1. Added "volatile long batchId = -1" field to InflightEntry
   - Stores the batch ID assigned by flushItems() so cancel() can
     locate and repack the correct batch entry.
   - Volatile ensures cross-thread visibility without synchronization.
   - -1 sentinel means batch not yet committed; repack is skipped.

2. Set entry.batchId in flushItems() after commitBatch()
   - Must be AFTER commitBatch (so repackBatch finds the entry in
     PrefillEndpoint.inflightBatches) and BEFORE dispatch (so cancel
     during gRPC can also repack).

3. Added repackPrefillBatch(InflightEntry) helper method
   - Calls PrefillEndpoint.repackBatch(batchId, {requestId}) which:
     - Single-request batch → removes entire entry (batch empty).
     - Multi-request batch → keeps survivors, removes only this request.
     - Batch already gone (calibrate/releaseBatch ran first) → no-op.
   - Idempotent via ConcurrentHashMap.computeIfPresent.

4. Called repackPrefillBatch in three cancel paths:
   - cancel(): main client-cancel path (timeout or upstream abort).
   - onSuccess() cancelAfterAck: server reports cancel before ack.
   - InflightEvictor callback: TTL eviction (default 300s) cleanup.

Edge cases handled:
- batchId=-1 (not yet committed): repack skipped, calibrate cleans up.
- Double repack (cancel + TTL): repackBatch is idempotent (no-op if
  batch already removed).
- Concurrent repack + releaseBatch: ConcurrentHashMap serializes.
…, and decode inflight

- Split mixed `app.engine.health.check.running.task.info.size` metric into:
  - `app.flexlb.batch.inflight.count` (prefill batch count, scheduler view)
  - `app.flexlb.batch.inflight.request.count` (prefill request count, scheduler view)
- Add decode-side inflight metrics:
  - `app.flexlb.decode.inflight.count` (decode inflight requests, scheduler view)
  - `app.flexlb.decode.total.load` (decode total load: confirmed + inflight)
- Add DecodeEndpoint.reportBatchMetrics() and EndpointRegistry.getDecodeEndpoints()
- FlexlbBatchScheduler.reportBatchMetrics() now iterates both prefill and decode endpoints
- BatchSchedulerReporter registers 13 metrics (was 10)
@LLLLKKKK

LLLLKKKK commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/0 · P1/4 · P2/2 · P3/0

Blocking Issues

P1

  • ResponseBufferWriter 在队列满时静默丢弃最老 output 会破坏流式响应完整性 @ rtp_llm/cpp/model_rpc/ResponseBuffer.cc:125
    • 建议:在队列满时应改为背压或失败:至少将 entry->cancelled.store(true) + 设置 entry->error_status = grpc::Status(RESOURCE_EXHAUSTED, "response buffer overflow")return false,让 PrefillRpcServer::EnqueueGroupfinish_lambda 通过 markResponseEntryDone 把错误传给 FetchResponse 消费端;或者在 kMaxQueueSize 加大到与最大 max_new_tokens 匹配前,至少要 kmonitor 记录一个 response_buffer_drop_qps 计数并 warning 日志一次以便发现。当前实现让流式请求可能悄无声息地少 token,属于典型的 silent-drop 反模式。
  • EnqueueGroup 中 pushTask 失败后回退 detached thread 的 std::move 存在 use-after-move 风险 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:1780
    • 建议:finish_lambda 应先构造为 std::shared_ptr<std::function<void()>>,先做 pushTask 尝试;失败后再基于同一份未被 move-out 的实体启动 fallback 线程。参考模式:
auto task = std::make_shared<std::function<void()>>(std::move(finish_lambda));
auto error = slot_worker_pool_->pushTask([task] { (*task)(); });
if (error != autil::ThreadPoolBase::ERROR_NONE) {
...
std::thread fallback_thread([task] { (*task)(); });
fallback_thread.detach();
}

或者在 pushTask 之前先 copy 一份 auto backup = finish_lambda; 用于 fallback。同时给 fallback 线程加 try/catch,避免任何未来的空 std::function 直接终结进程;并把 response_worker_count_ 泄漏路径通过 test(例如 mock 线程池 always return ERROR_QUEUE_FULL)覆盖到 stopAsyncResponseWorkers 场景。

  • WorkerStatusPB.role 从 string 改为 RoleTypePB 是同 tag 的 wire-incompatible 变更 @ rtp_llm/cpp/model_rpc/proto/model_rpc_service.proto:428
    • 建议:- 至少写清升级顺序(README / release notes):所有 flexlb + engine + 监控组件必须同时升级,且不允许旧版本 engine 与新版本 master 共存;并在 master_client.py 或对应 java 侧加显式的版本自检(例如 WorkerStatusPB 里加一个 int32 proto_version = 20,旧客户端读到 0 时降级)。
  • 若需要保留兼容性,标准做法是保留旧 string role = 1; 字段并新增 RoleTypePB role_type = 16;,两者共存一段时间后再废弃旧 tag;或者新加 WorkerStatusV2PB 消息与新 RPC 并存。
  • 至少在 CHANGELOG/PR description 明确列出这次是 breaking wire change,避免升级时出现无法定位的 decode failure。
  • is_backend_service_ready 整段被注释、无条件返回 True,健康探测被短路 @ rtp_llm/server/backend_rpc_server_visitor.py:549
    • 建议:- 至少改为显式开关:在 MasterConfig 增加 bypass_backend_ready_check: bool = False,默认保留旧健康检查语义;只在 FlexLB 强制路由模式(例如 master_config.disable_domain_fallback=True)下才短路 readiness,并 log 一次 warning。
  • 删除大段注释代码,只保留最终逻辑分支;或恢复原来的健康检查,仅在 role_addrs 为空且 FlexLB 有效时补一个 "FlexLB 路由可用" 兜底判定。
  • 建议同步补 unit test 验证:backend 全部下线 → readiness 应为 False;FlexLB 通路存活 → readiness 为 True。

Non-blocking Suggestions

P2

  • PR 合入 202 文件、33 commit、跨 C++/Python/Java/proto,未拆分,回滚粒度过大 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:1
    • 建议:按可独立回滚的边界拆分:
  1. proto schema breaking change(含 WorkerStatusPB.roleRoleTypePB 提升、batch_group_*→group_* rename)单独一 PR,先在客户端/服务端做兼容 shim;
  2. FlexLB Java 端与 master_client gRPC 迁移一起一 PR;
  3. ScheduleUnit + GenerateStream::prepare/isReady/activate/advance/finish 生命周期重构一 PR;
  4. PrefillRpcServer 三线程池 + ResponseBufferRegistry + EnqueueBatch/EnqueueGroup/FetchResponse/Cancel 一 PR;
  5. RpcCpuTpBroadcaster 单独一 PR;
  6. 剩余日志/hash 修复类改动可合并成一 PR。

PR 描述里应至少写出:动机、设计要点、rollout 顺序、兼容性影响、性能预期变化,方便 code review 与后续复盘。

  • PrefillRpcServer theory-hit 日志默认写死内部固定路径且默认启用 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:270
    • 建议:- prefillTheoryHitLogEnabled 默认改为 false(require opt-in),或者只在 PrefillCacheHitMetricsReporter::enabled() 时才启用;
  • 默认路径改为可移植位置(例如 ${TMPDIR:-/tmp}/rtp_llm_prefill_theory_hit.log),或强制要求环境变量;
  • TheoryHitStats 移入 PrefillRpcServer 成员,与 recent_cache_key_window 生命周期一致;
  • 打开失败后若干分钟后重试一次,避免运维恢复 ************logs 后仍然一直静默。

Checklist Violations (8 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 兼容性:公开 API/持久数据/配置/环境迁移安全 → issue WorkerStatusPB.role 从 string 改为 RoleTypePB 是同 tag 的 wire-incompatible 变更
    proto 变更:
  • [6.1] Architecture — 可观测性:日志/指标/超时可操作、非噪声 → issue PrefillRpcServer theory-hit 日志默认写死内部固定路径且默认启用
    prefillTheoryHitLogPath 默认 "************logs/prefill_theory_hit.log"prefillTheoryHitLogEnabledPREFILL_THEORY_HIT_LOG_ENABLED 未设置时返回 true。也就是任意机器(含 CI/开源用户/PPU/rocm 容器)一旦跑 PrefillRpcServer 都会尝试打开这个 hardcoded 内部路径,写请求级别的 request_id/request_key/cache_key_digest/hit_ratio 明文到磁盘,且只在打开失败时 warning 一次后 open_failed 永久 true,之后完全不再重试也不再上报。带来几个问题:(1)内部路径耦合,************logs 是内部机器约定,在开源用户或其他容器里不可用,会看到一条一次性 warning 后无声失败;(2)请求级 request_key 落盘可能包含内部业务 trace_id,需评估合规;(3)theory_stats 在 `report
  • [6.1] Architecture — 回滚路径:风险行为存在运维回滚手段 → issue PrefillRpcServer theory-hit 日志默认写死内部固定路径且默认启用
    prefillTheoryHitLogPath 默认 "************logs/prefill_theory_hit.log"prefillTheoryHitLogEnabledPREFILL_THEORY_HIT_LOG_ENABLED 未设置时返回 true。也就是任意机器(含 CI/开源用户/PPU/rocm 容器)一旦跑 PrefillRpcServer 都会尝试打开这个 hardcoded 内部路径,写请求级别的 request_id/request_key/cache_key_digest/hit_ratio 明文到磁盘,且只在打开失败时 warning 一次后 open_failed 永久 true,之后完全不再重试也不再上报。带来几个问题:(1)内部路径耦合,************logs 是内部机器约定,在开源用户或其他容器里不可用,会看到一条一次性 warning 后无声失败;(2)请求级 request_key 落盘可能包含内部业务 trace_id,需评估合规;(3)theory_stats 在 `report
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue EnqueueGroup 中 pushTask 失败后回退 detached thread 的 std::move 存在 use-after-move 风险
    start_finish_worker 里的 fallback 分支代码:
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue is_backend_service_ready 整段被注释、无条件返回 True,健康探测被短路
    ```python
  • [6.1] Quality — Commit 原子、message 与行为匹配 → issue PR 合入 202 文件、33 commit、跨 C++/Python/Java/proto,未拆分,回滚粒度过大
    单 PR 涉及 202 个 focus_files(含 flexlb 大量 Java 改造 / 新增线程池 / 状态机重构 / proto schema breaking change / master gRPC 迁移 / 新增 CPU TP 广播组件 / KVCache hash 修复 / 大量日志追加),diff ~30k 行;PR body 为空。这类 mega-PR 在 CI 阶段一旦一个模块回归很难 bisect 到具体 commit(例如 FIFOScheduler admitWaitingUnits 的 admitted_total_tokens 语义变化:现在 loading 单元也计入 token 预算,与旧注释「loading_cache_streams 这一轮实际不参与计算」不一致,属于潜在吞吐回退,但混在 200 个文件里很难被 review 注意到)。
  • [6.1] Quality — Mega-PR 已拆分为独立变更 → issue PR 合入 202 文件、33 commit、跨 C++/Python/Java/proto,未拆分,回滚粒度过大
    单 PR 涉及 202 个 focus_files(含 flexlb 大量 Java 改造 / 新增线程池 / 状态机重构 / proto schema breaking change / master gRPC 迁移 / 新增 CPU TP 广播组件 / KVCache hash 修复 / 大量日志追加),diff ~30k 行;PR body 为空。这类 mega-PR 在 CI 阶段一旦一个模块回归很难 bisect 到具体 commit(例如 FIFOScheduler admitWaitingUnits 的 admitted_total_tokens 语义变化:现在 loading 单元也计入 token 预算,与旧注释「loading_cache_streams 这一轮实际不参与计算」不一致,属于潜在吞吐回退,但混在 200 个文件里很难被 review 注意到)。
  • [6.1] Quality — PR description 说明动机与设计 → issue PR 合入 202 文件、33 commit、跨 C++/Python/Java/proto,未拆分,回滚粒度过大
    单 PR 涉及 202 个 focus_files(含 flexlb 大量 Java 改造 / 新增线程池 / 状态机重构 / proto schema breaking change / master gRPC 迁移 / 新增 CPU TP 广播组件 / KVCache hash 修复 / 大量日志追加),diff ~30k 行;PR body 为空。这类 mega-PR 在 CI 阶段一旦一个模块回归很难 bisect 到具体 commit(例如 FIFOScheduler admitWaitingUnits 的 admitted_total_tokens 语义变化:现在 loading 单元也计入 token 预算,与旧注释「loading_cache_streams 这一轮实际不参与计算」不一致,属于潜在吞吐回退,但混在 200 个文件里很难被 review 注意到)。

Strengths

  • ScheduleUnit 抽象把 stream lifecycle 从 scheduler 中央 switch/case 集中到 stream 自身的 prepare/isReady/activate/advance/finish,配合 hasPendingAsyncBookkeeping 的 deferred release 逻辑,比原来的 moveToNext + handleWaiting/Loading/Running 更内聚,也让 DecodeRpcServer::allocateResource 的 busy-wait 代码含义更明确。
  • PrefillRpcServer 的 L1/L2/L3/L4 分层线程池 + PoolMetrics 定期上报(active/queued/completed/rejected/fallback),运行态可观测性明显改善;ResponseBufferRegistry::gc 有 TTL + 显式 cancel_producer 回调,异步生命周期收敛机制清晰。
  • KVCacheHashUtil::updateCacheKeys 补充了对 partial block key 的重算,并修正 setLastBlockAlignedinitCacheKeys 语义一致,修复了短序列 device cache 无法插入的历史 bug。
  • 新增 ResponseBufferTestRpcServerRuntimeMetaTestRpcCpuTpBroadcasterTestFIFOSchedulerAsyncCacheTestFIFOSchedulerCancelTestFIFOSchedulerTest 覆盖了关键新增行为。
  • Proto tag 复用做了 reserved 55; 处理 force_batch 字段的废弃,避免同 tag 类型再冲突。

wzy-99 and others added 4 commits July 3, 2026 19:58
…etrics reporting frequency

- Replace hardcoded lowercase role strings ('prefill'/'decode') with
  RoleType.PREFILL.name()/RoleType.DECODE.name() across BatchSchedulerReporter,
  PrefillEndpoint, FixedWindowBatcherAlgorithm, and FlexlbBatchScheduler
- Aligns scheduler-side metrics with EngineHealthReporter's role naming
  convention (uppercase enum name) for consistent Grafana filtering
- Increase reportBatchMetrics frequency from 20s to 2s for finer granularity
…ch_end for one-shot extraction

- begin: keep only ctx_batch/gen_batch/total_tokens/max_seq (brief marker)
- end: add full streams detail + gen_batch/max_seq/forward_us (complete info)
- add trace_id to MtpExecutor streams format (was missing, now matches NormalExecutor)
- enables single grep prefill_batch_end to get batch composition + timing
@CLAassistant

CLAassistant commented Jul 4, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
3 out of 4 committers have signed the CLA.

✅ xinfei-shi
✅ wzy-99
✅ jianglan89
❌ Codex


Codex seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@LLLLKKKK

LLLLKKKK commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

AI Code Review - PR #1138

Status: BLOCKING

Summary: P0/0 · P1/2 · P2/5 · P3/0

Blocking Issues

P1

  • FIFOScheduler 重写静默丢弃 cp_force_single_prefill / max_inited_kv_cache_streams 强制逻辑 @ rtp_llm/cpp/engine_base/schedulers/FIFOScheduler.cc:187
    • 建议:明确取舍:若确实不再需要这两个开关,应同步删除 FIFOSchedulerConfig 中的字段、pybind 绑定、pickle 项与相关单测,避免配置成为静默 no-op;若仍需保留 CP prefill 单条与 KV 初始化并发上限的语义,请在 canAdmitUnit/admitWaitingUnits 中补回对应判断并让单测通过。无论哪种,都需消除“实现忽略配置但单测断言旧行为”的矛盾。
  • WorkerStatusPB.role 由 string 改为 enum 复用同一字段号,滚动升级线格式不兼容 @ rtp_llm/cpp/model_rpc/proto/model_rpc_service.proto:431
    • 建议:按 proto 演进规范处理:reserved 1;(连同旧名)并为枚举 role 分配一个新的字段号,保证新旧二进制在滚动升级期间可互相跳过未知/旧字段而不误读。若确认引擎与所有消费者严格同版本原子发布、不存在混版窗口,请在 PR 描述中显式说明该假设与发布顺序约束。

Non-blocking Suggestions

P2

  • prefill 近期 cache-key / theory-hit 指标为死代码,从未被调用 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:1280
    • 建议:若该指标特性本应生效,请在合适阶段(如 finishStream/pollLocalOutput 完成后)真正调用 reportPrefillRecentCacheKeyMetricsOnce,并补充触发测试;若暂不启用,建议移除或用编译/运行开关隔离这批死代码。同时:日志默认值改为关闭、日志路径改为可配置且避免硬编码内部绝对路径,硬编码的路径常量与 reporter 中的重复项应统一到一处。
  • prefill_stop_stream_wait_timeout_ms 未打通 pybind,server arg 被静默忽略 @ rtp_llm/cpp/pybind/ConfigInit.cc:1697
    • 建议:在 ConfigInit.ccPDSepConfig 增加 def_readwrite("prefill_stop_stream_wait_timeout_ms", ...),并把该字段补入 pickle 的 getstate/setstate(同步调整 t.size() 校验与索引)。另建议评估 _apply_config_bindings 对绑定失败仅 warning 的做法:对“配置成功注册但目标对象无此属性”的情况应尽量 fail-fast 或显著告警,避免运维以为设置生效实则被静默丢弃。
  • ResponseBufferWriter 队列满时丢弃队首,流式输出可能被静默丢失 @ rtp_llm/cpp/model_rpc/ResponseBuffer.cc:125
    • 建议:明确背压/丢弃语义:优先考虑写满时对该请求置错(如 RESOURCE_EXHAUSTED)并终止,而非静默丢数据;确需环形缓冲时,应在丢弃时计数并上报指标/日志,且需确认输出为累积语义时丢中间帧才安全。请在代码注释与 PR 描述中说明所选语义及其对最终结果正确性的影响。
  • EnqueueGroup 放弃未完成 future 与超时回调并发写 LocalSlot,存在数据竞争/复位后使用 @ rtp_llm/cpp/model_rpc/PrefillRpcServer.cc:2036
    • 建议:为放弃的 future 建立明确的所有权/同步:例如让每个 slot 任务持有自身 shared_ptr 生命周期、以原子/状态位标记“已被协调线程接管”,在 fail_slot reset 前确保对应任务不再触碰该 slot(或不 detach、改为可取消并等待其退出)。同时 on_timeout 不应与仍在运行的任务写同一字段,建议改为写入独立的、由单一线程拥有的结果槽位。
  • w4a8 量化在异常处理里使用未导入的 logging,掩盖真实 ImportError @ rtp_llm/model_loader/w4a8_int4_per_channel_quant_weight.py:31
    • 建议:在文件顶部补 import logging(并注意 isort/black 排序)。也可在此直接 raise ImportError(...) from e 以保留原始异常链,避免仅靠 warning + 裸 raise。

Checklist Violations (6 fail / 56 total)

General Principles Checklist

  • [6.1] Architecture — 兼容性:公开 API/持久数据/配置/环境迁移安全 → issue prefill_stop_stream_wait_timeout_ms 未打通 pybind,server arg 被静默忽略
    PDSepConfig 新增字段 prefill_stop_stream_wait_timeout_ms(C++ 侧确在 PrefillGenerateContext 的 stopStream 等待循环中使用),并在 pd_separation_group_args.py 注册了 --prefill_stop_stream_wait_timeout_msbind_to=(pd_separation_config, "prefill_stop_stream_wait_timeout_ms"))。但 ConfigInit.ccpy::class_<PDSepConfig>(:1697)未加该字段的 def_readwrite(同批次其余 6 个 pool/timeout 字段都加了),该类也未开启 py::dynamic_attr(),且 pickle 的 getstate/setstate 仅含 6 个新字段、未含本字段。因此 setattr 会抛 AttributeError,被 `server_args.apply_config
  • [6.1] Architecture — 可观测性:日志/指标/超时可操作、非噪声 → issue prefill 近期 cache-key / theory-hit 指标为死代码,从未被调用
    init() 中按 PrefillCacheHitMetricsReporter::enabled() 分配了 prefill_recent_cache_key_window_,但唯一使用它的入口 reportPrefillRecentCacheKeyMetricsOnce(:1280)在整个代码树中没有任何调用点(仅声明+定义)。随之新增的 TheoryHitStatsbuildFullBlockCacheKeysappendPrefillTheoryHitLogLinefillPrefill*Collector 等数百行机制全部不可达,指标永远不会上报。此外该日志落盘函数默认开启(prefillTheoryHitLogEnabled() 无 env 时返回 true)并硬编码了一个内部绝对日志路径(PrefillRpcServer.cc:112),一旦将来接线,会在请求路径上做带 flush 的单文件追加,且该常量与 PrefillCacheHitMetricsReporter.cc 重复。
  • [6.1] Architecture — 状态不变量:创建/更新/失败/重试/回滚路径有效 → issue EnqueueGroup 放弃未完成 future 与超时回调并发写 LocalSlot,存在数据竞争/复位后使用
    prepare/load 阶段将任务提交至 slot_worker_pool_,lambda 捕获 slot_ptr=&slot(指向 *slots_ptr)并写 slot.stage_status/slot.prepared/slot.prefill_contextcollectFutures 的 on_timeout 回调在协调线程写 slots[i].stage_status = DEADLINE_EXCEEDED(:2033),此时对应 lambda 可能仍在运行并写同一字段,构成竞争;随后 drainReadyFutures(2000ms)detachLeftoverFutures(:2036)直接放弃仍未就绪的 future,被放弃的 lambda 继续访问 slot.prefill_context,而协调线程对未 prepared 的 slot 走 fail_slotreset() 这些成员。在线程池饱和(代码本身有 rejected/fallback 计数,说明饱和是预期)+ 超时叠加时,可能出现对已 reset
  • [6.1] Architecture — 错误语义:fail-fast/retry/fallback/silent 行为显式 → issue w4a8 量化在异常处理里使用未导入的 logging,掩盖真实 ImportError
    quantize_weight_to_int4bexcept ImportError as e: logging.warning(...) (:31),但该模块顶部只 import 了 copytyping.Optionaltorch 及 rtp_llm 相关模块,并未 import logging。当 rtp_kernel.w4a8_group_gemm 不可用触发 ImportError 时,logging.warning 会先抛出 NameError: name 'logging' is not defined,从而以 NameError 覆盖原始 ImportError,运维排查 W4A8 内核缺失时会看到误导性的报错。仅影响内核不可用的错误路径,happy path 不受影响。
  • [6.1] Software Engineering — KISS/YAGNI:无投机性抽象 → issue prefill 近期 cache-key / theory-hit 指标为死代码,从未被调用
    init() 中按 PrefillCacheHitMetricsReporter::enabled() 分配了 prefill_recent_cache_key_window_,但唯一使用它的入口 reportPrefillRecentCacheKeyMetricsOnce(:1280)在整个代码树中没有任何调用点(仅声明+定义)。随之新增的 TheoryHitStatsbuildFullBlockCacheKeysappendPrefillTheoryHitLogLinefillPrefill*Collector 等数百行机制全部不可达,指标永远不会上报。此外该日志落盘函数默认开启(prefillTheoryHitLogEnabled() 无 env 时返回 true)并硬编码了一个内部绝对日志路径(PrefillRpcServer.cc:112),一旦将来接线,会在请求路径上做带 flush 的单文件追加,且该常量与 PrefillCacheHitMetricsReporter.cc 重复。
  • [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue FIFOScheduler 重写静默丢弃 cp_force_single_prefill / max_inited_kv_cache_streams 强制逻辑
    重写后 FIFOScheduler 删除了成员 cp_force_single_prefill_max_inited_kv_cache_streams_canAdmitUnit(:187)对 PREFILL 只判断 running_count>0max_generate_batch_size 与 token 预算,不再限制 CP prefill 每轮单条、也不再限制并发 KV 初始化条数。但这两个配置仍在 FIFOSchedulerConfig/pybind/pickle 中暴露,且保留的单测仍断言旧语义:testCpForceSinglePrefillConfig 断言 schedule_two_prefills(true)==1(FIFOSchedulerTest.cc:1144),新逻辑对两条非 group prefill 会同时准入返回 2;testMaxInitedKVCacheStreamsBlocksNewInit(:98)断言只准入 1 条、第二条 curBlocksNum()==0 且仍在 waiting,新逻辑无

Strengths

  • 线程池(LockFreeThreadPool)替代原先裸 std::async/std::thread::detach,并配套 active/queued/completed/rejected/fallback 的 PoolMetrics 与 kmonitor 上报,可观测性明显增强。
  • 大量使用 ScopeExit RAII 守卫来保证计数递减与句柄释放,异常路径的资源回收考虑较周全。
  • proto 对删除字段使用了 reservedforce_batch 的 55、TaskInfoPB.is_waiting 的 9),符合演进规范。
  • ScheduleUnit 分组调度、ResponseBuffermaster_client gRPC 化均补充了聚焦单测(group isolation、ResponseBufferTestmaster_client_test 等)。
  • prefill 批量通路对取消(AsyncProducerCancelState + cancel_producer + Cancel RPC)做了端到端传播设计。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants